[rules-dev] Bug in BetaNode#doRemove()?

[Edson Tirelli]

  Hi Andreas,

   Yes, please open a JIRA and attach your patch. I will try to reproduce
and create a test case from your description, but sometimes it would be
faster if you could provide a dumbed down or obfuscated version of the rule
you are using that is causing the problem...

   Thanks,
     Edson

2010/3/29 Andreas Kohn <andreas.kohn at fredhopper.com>

> Hi,
>
> 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.
>
>
> Should I open a JIRA issue? Unfortunately I cannot provide a DRL to
> reproduce the issue, but I could
> try dumping the rulebase if that would help.
>
>
> Regards,
> --
> Andreas
>
> --
> Never attribute to malice that which can be adequately explained by
> stupidity.                                        -- Hanlon's Razor
>
> _______________________________________________
> rules-dev mailing list
> rules-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-dev
>
>


-- 
 Edson Tirelli
 JBoss Drools Core Development
 JBoss by Red Hat @ www.jboss.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/rules-dev/attachments/20100330/308c1b83/attachment-0001.html