[rules-dev] Bug in BetaNode#doRemove()?
Andreas Kohn
andreas.kohn at fredhopper.com
Wed Mar 31 04:00:24 EDT 2010
On Tue, 2010-03-30 at 13:00 -0400, Edson Tirelli wrote:
>
> 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...
>
I created https://jira.jboss.org/jira/browse/JBRULES-2465
We're currently in a release crunch, so I can't get you a self-contained
test right now, but I'll try to get around to it.
Thanks,
--
Andreas
> 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
> _______________________________________________
> rules-dev mailing list
> rules-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-dev
--
Never attribute to malice that which can be adequately explained by
stupidity. -- Hanlon's Razor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
Url : http://lists.jboss.org/pipermail/rules-dev/attachments/20100331/42de3d91/attachment-0001.bin
More information about the rules-dev
mailing list