Skip to content

Commit

Permalink
[BZ-1228313] prevent NPE during a right update when no left memory is…
Browse files Browse the repository at this point in the history 
… present (sequential mode)
  • Loading branch information
@mariofusco
mariofusco committed Jun 4, 2015
1 parent 89d08fd commit 87ee53c
Show file tree
Hide file tree
Showing 10 changed files with 342 additions and 343 deletions.
30 changes: 30 additions & 0 deletions ...ompiler/src/test/java/org/drools/compiler/integrationtests/sequential/SequentialTest.java
View file
Expand Up @@ -28,6 +28,7 @@
import org.kie.api.event.rule.RuleFlowGroupActivatedEvent;
import org.kie.api.event.rule.RuleFlowGroupDeactivatedEvent;
import org.kie.api.event.rule.RuleRuntimeEventListener;
import org.kie.api.io.ResourceType;
import org.kie.api.runtime.KieContainer;
import org.kie.api.runtime.StatelessKieSession;
import org.kie.internal.KnowledgeBase;
Expand All @@ -36,6 +37,7 @@
import org.kie.internal.command.CommandFactory;
import org.kie.internal.conf.SequentialOption;
import org.kie.internal.runtime.StatelessKnowledgeSession;
import org.kie.internal.utils.KieHelper;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -538,4 +540,32 @@ public void testSequentialWithNoLoop() throws Exception {
result = (List) ksession.execute( CommandFactory.newInsertElements(Arrays.asList("test", new Message())));
assertEquals( 2, result.size() );
}

@Test
public void testSharedSegment() throws Exception {
// BZ-1228313
String str =
"package org.drools.compiler.test\n" +
"import \n" + Message.class.getCanonicalName() + ";" +
"rule R1 when\n" +
" $s : String()\n" +
" $m : Message()\n" +
" $i : Integer( this < $s.length )\n" +
"then\n" +
" modify($m) { setMessage($s) };\n" +
"end\n" +
"\n" +
"rule R2 when\n" +
" $s : String()\n" +
" $m : Message()\n" +
" $i : Integer( this > $s.length )\n" +
"then\n" +
"end\n";

StatelessKieSession ksession = new KieHelper().addContent( str, ResourceType.DRL )
.build( SequentialOption.YES )
.newStatelessKieSession();

ksession.execute( CommandFactory.newInsertElements(Arrays.asList("test", new Message(), 3, 5)));
}
}
153 changes: 71 additions & 82 deletions drools-core/src/main/java/org/drools/core/phreak/PhreakAccumulateNode.java
View file
Expand Up @@ -94,9 +94,9 @@ public void doNode(AccumulateNode accNode,

for (LeftTuple leftTuple = tempLeftTuples.getUpdateFirst(); leftTuple != null; ) {
LeftTuple next = leftTuple.getStagedNext();
evaluateResultConstraints(accNode, sink, accumulate, leftTuple, leftTuple.getPropagationContext(),
wm, am, (AccumulateContext) leftTuple.getObject(),
trgLeftTuples, stagedLeftTuples);
evaluateResultConstraints( accNode, sink, accumulate, leftTuple, leftTuple.getPropagationContext(),
wm, am, (AccumulateContext) leftTuple.getObject(),
trgLeftTuples, stagedLeftTuples );
leftTuple.clearStaged();
leftTuple = next;
}
Expand Down Expand Up @@ -144,9 +144,9 @@ public void doLeftInserts(AccumulateNode accNode,
leftTuple,
wm);

constraints.updateFromTuple(contextEntry,
wm,
leftTuple);
constraints.updateFromTuple( contextEntry,
wm,
leftTuple );

FastIterator rightIt = accNode.getRightIterator(rtm);

Expand Down Expand Up @@ -181,13 +181,13 @@ public void doLeftInserts(AccumulateNode accNode,
}

leftTuple.clearStaged();
trgLeftTuples.addInsert(leftTuple);
trgLeftTuples.addInsert( leftTuple );

constraints.resetTuple(contextEntry);
constraints.resetTuple( contextEntry );

leftTuple = next;
}
constraints.resetTuple(contextEntry);
constraints.resetTuple( contextEntry );
}

public void doRightInserts(AccumulateNode accNode,
Expand All @@ -209,50 +209,43 @@ public void doRightInserts(AccumulateNode accNode,

for (RightTuple rightTuple = srcRightTuples.getInsertFirst(); rightTuple != null; ) {
RightTuple next = rightTuple.getStagedNext();
rtm.add( rightTuple );

rtm.add(rightTuple);
if ( ltm == null || ltm.size() == 0 ) {
// do nothing here, as no left memory
rightTuple.clearStaged();
rightTuple = next;
continue;
}

PropagationContext context = rightTuple.getPropagationContext();
constraints.updateFromFactHandle(contextEntry,
wm,
rightTuple.getFactHandle());
if ( ltm != null && ltm.size() > 0 ) {
constraints.updateFromFactHandle( contextEntry,
wm,
rightTuple.getFactHandle() );

FastIterator leftIt = accNode.getLeftIterator(ltm);
FastIterator leftIt = accNode.getLeftIterator( ltm );

for (LeftTuple leftTuple = accNode.getFirstLeftTuple(rightTuple, ltm, context, leftIt); leftTuple != null; leftTuple = (LeftTuple) leftIt.next(leftTuple)) {
if (constraints.isAllowedCachedRight(contextEntry,
leftTuple)) {
final AccumulateContext accctx = (AccumulateContext) leftTuple.getObject();
addMatch(accNode,
accumulate,
leftTuple,
rightTuple,
null,
null,
wm,
am,
accctx,
true);
for ( LeftTuple leftTuple = accNode.getFirstLeftTuple( rightTuple, ltm, leftIt ); leftTuple != null; leftTuple = (LeftTuple) leftIt.next( leftTuple ) ) {
if ( constraints.isAllowedCachedRight( contextEntry,
leftTuple ) ) {
final AccumulateContext accctx = (AccumulateContext) leftTuple.getObject();
addMatch( accNode,
accumulate,
leftTuple,
rightTuple,
null,
null,
wm,
am,
accctx,
true );

// right inserts and updates are done first
// so any existing leftTuples we know are updates, but only add if not already added
if ( leftTuple.getStagedType() == LeftTuple.NONE ) {
trgLeftTuples.addUpdate( leftTuple );
}

// right inserts and updates are done first
// so any existing leftTuples we know are updates, but only add if not already added
if (leftTuple.getStagedType() == LeftTuple.NONE) {
trgLeftTuples.addUpdate(leftTuple);
}

}
}

rightTuple.clearStaged();
rightTuple = next;
}
constraints.resetFactHandle(contextEntry);
constraints.resetFactHandle( contextEntry );
}

public void doLeftUpdates(AccumulateNode accNode,
Expand Down Expand Up @@ -421,50 +414,50 @@ public void doRightUpdates(AccumulateNode accNode,

for (RightTuple rightTuple = srcRightTuples.getUpdateFirst(); rightTuple != null; ) {
RightTuple next = rightTuple.getStagedNext();
PropagationContext context = rightTuple.getPropagationContext();

LeftTuple childLeftTuple = rightTuple.getFirstChild();
if ( ltm != null && ltm.size() > 0 ) {
LeftTuple childLeftTuple = rightTuple.getFirstChild();

FastIterator leftIt = accNode.getLeftIterator(ltm);
LeftTuple leftTuple = accNode.getFirstLeftTuple(rightTuple, ltm, context, leftIt);
FastIterator leftIt = accNode.getLeftIterator( ltm );
LeftTuple leftTuple = accNode.getFirstLeftTuple( rightTuple, ltm, leftIt );

constraints.updateFromFactHandle(contextEntry,
wm,
rightTuple.getFactHandle());
constraints.updateFromFactHandle( contextEntry,
wm,
rightTuple.getFactHandle() );

// first check our index (for indexed nodes only) hasn't changed and we are returning the same bucket
// We assume a bucket change if leftTuple == null
if ( childLeftTuple != null && ltm.isIndexed() && !leftIt.isFullIterator() && ( leftTuple == null || ( leftTuple.getMemory() != childLeftTuple.getLeftParent().getMemory() ) ) ) {
// our index has changed, so delete all the previous matches
removePreviousMatchesForRightTuple( accNode,
accumulate,
rightTuple,
wm,
am,
childLeftTuple,
trgLeftTuples );
childLeftTuple = null; // null so the next check will attempt matches for new bucket
}

// first check our index (for indexed nodes only) hasn't changed and we are returning the same bucket
// We assume a bucket change if leftTuple == null
if (childLeftTuple != null && ltm.isIndexed() && !leftIt.isFullIterator() && (leftTuple == null || (leftTuple.getMemory() != childLeftTuple.getLeftParent().getMemory()))) {
// our index has changed, so delete all the previous matches
removePreviousMatchesForRightTuple(accNode,
// if LeftTupleMemory is empty, there are no matches to modify
if ( leftTuple != null ) {
if ( leftTuple.getStagedType() == LeftTuple.NONE ) {
trgLeftTuples.addUpdate( leftTuple );
}

doRightUpdatesProcessChildren( accNode,
am,
wm,
bm,
constraints,
accumulate,
leftIt,
rightTuple,
wm,
am,
childLeftTuple,
trgLeftTuples);
childLeftTuple = null; // null so the next check will attempt matches for new bucket
}

// if LeftTupleMemory is empty, there are no matches to modify
if (leftTuple != null) {
if (leftTuple.getStagedType() == LeftTuple.NONE) {
trgLeftTuples.addUpdate(leftTuple);
leftTuple,
trgLeftTuples );
}

doRightUpdatesProcessChildren(accNode,
am,
wm,
bm,
constraints,
accumulate,
leftIt,
rightTuple,
childLeftTuple,
leftTuple,
trgLeftTuples);
}

rightTuple.clearStaged();
rightTuple = next;
}
Expand Down Expand Up @@ -575,7 +568,6 @@ public void doLeftDeletes(AccumulateNode accNode,
LeftTupleSets trgLeftTuples) {
BetaMemory bm = am.getBetaMemory();
LeftTupleMemory ltm = bm.getLeftTupleMemory();
//ContextEntry[] contextEntry = bm.getContext();
Accumulate accumulate = accNode.getAccumulate();

for (LeftTuple leftTuple = srcLeftTuples.getDeleteFirst(); leftTuple != null; ) {
Expand All @@ -597,9 +589,6 @@ public void doLeftDeletes(AccumulateNode accNode,

if (accctx.propagated) {
trgLeftTuples.addDelete(accctx.resultLeftTuple);
} else {
// if not propagated, just destroy the result fact handle
// workingMemory.getFactHandleFactory().destroyFactHandle( accctx.result.getFactHandle() );
}
}

Expand Down

0 comments on commit 87ee53c

Please sign in to comment.