[jboss-jira] [JBoss JIRA] Commented: (JBRULES-2465) Corruption of Rete when removing complex NotNodes

[Andreas Kohn (JIRA)]

    [ https://jira.jboss.org/jira/browse/JBRULES-2465?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12526619#action_12526619 ] 

Andreas Kohn commented on JBRULES-2465:
---------------------------------------

Thanks for the explanations!

--
Andreas

> Corruption of Rete when removing complex NotNodes
> -------------------------------------------------
>
>                 Key: JBRULES-2465
>                 URL: https://jira.jboss.org/jira/browse/JBRULES-2465
>             Project: Drools
>          Issue Type: Bug
>      Security Level: Public(Everyone can see) 
>          Components: drools-core
>    Affects Versions: 5.0.1.FINAL
>            Reporter: Andreas Kohn
>            Assignee: Edson Tirelli
>             Fix For: 5.1.0.M2
>
>         Attachments: drools-core-betanode-remove.diff, drools-core-betanode-remove.diff
>
>
> while working further on our drools integration we came across an odd
> exception when removing a particular rule:
> java.lang.IllegalArgumentException: Cannot remove a sink, when the list of sinks is null
>         at org.drools.reteoo.ObjectSource.removeObjectSink(ObjectSource.java:159)
>         at org.drools.reteoo.RightInputAdapterNode.doRemove(RightInputAdapterNode.java:217)
>         at org.drools.common.BaseNode.remove(BaseNode.java:95)
>         at org.drools.reteoo.BetaNode.doRemove(BetaNode.java:275)
>         at org.drools.common.BaseNode.remove(BaseNode.java:95)
>         at org.drools.reteoo.BetaNode.doRemove(BetaNode.java:280)
>         at org.drools.common.BaseNode.remove(BaseNode.java:95)
>         at org.drools.reteoo.RuleTerminalNode.doRemove(RuleTerminalNode.java:387)
>         at org.drools.common.BaseNode.remove(BaseNode.java:95)
>         at org.drools.reteoo.ReteooBuilder.removeRule(ReteooBuilder.java:237)
>         at org.drools.reteoo.ReteooRuleBase.removeRule(ReteooRuleBase.java:371)
>         at org.drools.common.AbstractRuleBase.removeRule(AbstractRuleBase.java:746)
> While stepping through the code it looked like the network was corrupt (there was indeed no 
> sinks on the ObjectSource, but the node calling removeObjectSink was still linked to it 
> and claiming it as source). 
> The rule itself contains multiple NotNodes, checking a condition that looks like this:
> not(not(Foo.v = X) and not(Foo.v = Y))
> I could track this down to some sort of "loop" in the rete that triggers this when the outer
> not node is removed.
> When removing BetaNode#doRemove() first walks along 'rightInput':
>         this.rightInput.remove( context,
>                                 builder,
>                                 this,
>                                 workingMemories );
> and eventually _in that call_ it also hits a node that is the direct 'leftInput' of the original beta node. 
> The removal marks that node as visited in the removal context, and when the 'rightInput.remove' returns to the
> beta node it does not visit the leftInput due to this condition in BetaNode#doRemove():
>         if ( !context.alreadyVisited( this.leftInput ) ) {
>             this.leftInput.remove( context,
>                                    builder,
>                                    this,
>                                    workingMemories );
>         }
> In other words: before the remove the BetaNode had another node that was both referenced directly as 'leftInput', 
> as well as an input to the 'rightInput'.
> The first removal of the rule "worked", and no exceptions happened. But: any further attempt to re-add the same rule and remove
> it again lead to the exception above.
> I was able to fix it with the attached patch, reproduced here:
> +        boolean needRemoveFromLeft = !context.alreadyVisited( this.leftInput );
>          this.rightInput.remove( context,
>                                  builder,
>                                  this,
>                                  workingMemories );
> -        if ( !context.alreadyVisited( this.leftInput ) ) {
> +        if ( needRemoveFromLeft ) {
>              this.leftInput.remove( context,
>                                     builder,
>                                     this,
>                                     workingMemories );
>          }
> With this patch applied I could add/delete/add the particular rule repeatedly without problems.
> The attached patch also adds an assert in ObjectSource#removeObjectSink(): when removing a sink from an object source with
> only one sink the sink was unconditionally replaced with an empty sink, although the argument ObjectSink could be a different
> sink than the one in the ObjectSource. For CompositeObjectSinkAdapters this case is checked, but not for single sinks.
> I originally suspected that place to be responsible for the problem I observed but the assertion never fired in my tests.
> (taken from rules-dev mail "Bug in BetaNode#doRemove()?"

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira