tkobayas commented on code in PR #6461:
URL:
https://github.com/apache/incubator-kie-drools/pull/6461#discussion_r2362373118
##########
drools-core/src/main/java/org/drools/core/phreak/PhreakBranchNode.java:
##########
@@ -166,11 +166,8 @@ public void doLeftUpdates(ConditionalBranchNode branchNode,
if (branchTuples.mainLeftTuple != null) {
normalizeStagedTuples( stagedLeftTuples,
branchTuples.mainLeftTuple );
- if (breaking &&
!NodeTypeEnums.isTerminalNode(branchTuples.mainLeftTuple.getSink())) {
- // child exist, new one does not, so delete
- trgLeftTuples.addDelete(branchTuples.mainLeftTuple);
- } else {
- // child exist, new one does, so update
+ if (!breaking) {
+ // default consequence will also be executed
trgLeftTuples.addUpdate(branchTuples.mainLeftTuple);
}
Review Comment:
@mariofusco
The current behaviour `if (breaking &&
!NodeTypeEnums.isTerminalNode(branchTuples.mainLeftTuple.getSink()))` updates
`branchTuples.mainLeftTuple`, so both a default consequence and a named
consequence are executed. This is the originally reported issue and the test
case reproduces that.
If I remove the condition `&&
!NodeTypeEnums.isTerminalNode(branchTuples.mainLeftTuple.getSink())`
(introduced by
https://github.com/apache/incubator-kie-drools/commit/87b2f656c495ef61c048db7cc3aedf2a0ed821bd),
it deletes `branchTuples.mainLeftTuple`. Then the next issue arises. If the
branch condition is evaluated as `default` -> `break` -> `default` (like the
test case). The deleted (= `removeDormantTuple` is called)
`branchTuples.mainLeftTuple` is updated, so `modifyActiveTuple` causes NPE
because it calls `removeDormantTuple` to a tuple which was already removed from
dormantMatches LinkedList.
```
java.lang.NullPointerException: Cannot invoke
"org.drools.core.util.DoubleLinkedEntry.setNext(org.drools.core.util.SingleLinkedEntry)"
because the return value of
"org.drools.core.util.DoubleLinkedEntry.getPrevious()" is null
at org.drools.core.util.LinkedList.remove(LinkedList.java:178)
at
org.drools.core.phreak.RuleExecutor.removeDormantTuple(RuleExecutor.java:318)
at
org.drools.core.phreak.RuleExecutor.modifyActiveTuple(RuleExecutor.java:338)
at
org.drools.core.phreak.PhreakRuleTerminalNode.doLeftTupleUpdate(PhreakRuleTerminalNode.java:223)
at
org.drools.core.phreak.PhreakRuleTerminalNode.doLeftUpdates(PhreakRuleTerminalNode.java:165)
at
org.drools.core.phreak.PhreakRuleTerminalNode.doNode(PhreakRuleTerminalNode.java:56)
at
org.drools.core.phreak.RuleNetworkEvaluator.innerEval(RuleNetworkEvaluator.java:335)
at
org.drools.core.phreak.RuleNetworkEvaluator.outerEval(RuleNetworkEvaluator.java:189)
```
stacktrace with `DEBUG_DORMANT_TUPLE = true`
```
java.lang.IllegalStateException
at
org.drools.core.phreak.RuleExecutor.removeDormantTuple(RuleExecutor.java:315)
at
org.drools.core.phreak.RuleExecutor.modifyActiveTuple(RuleExecutor.java:338)
at
org.drools.core.phreak.PhreakRuleTerminalNode.doLeftTupleUpdate(PhreakRuleTerminalNode.java:223)
at
org.drools.core.phreak.PhreakRuleTerminalNode.doLeftUpdates(PhreakRuleTerminalNode.java:165)
at
org.drools.core.phreak.PhreakRuleTerminalNode.doNode(PhreakRuleTerminalNode.java:56)
at
org.drools.core.phreak.RuleNetworkEvaluator.innerEval(RuleNetworkEvaluator.java:335)
at
org.drools.core.phreak.RuleNetworkEvaluator.outerEval(RuleNetworkEvaluator.java:189)
at
org.drools.core.phreak.RuleNetworkEvaluator.evaluateNetwork(RuleNetworkEvaluator.java:147)
```
Possible fixes:
A) Don't delete `mainLeftTuple`.
Update `branchTuples.mainLeftTuple` when `breaking == false`. Otherwise,
just leave it, so it can be updated when the condition results in `default` in
the future.
B) Completely delete `mainLeftTuple`.
`trgLeftTuples.addDelete(branchTuples.mainLeftTuple)` doesn't seem to be
enough. Probably we can remove the `mainLeftTuple` reference from
`branchTuples`.
Currently my fix is A). But if you think B) is the right direction, feel
free to let me know. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
