[jboss-jira] [JBoss JIRA] Commented: (JBRULES-2465) Corruption of Rete when removing complex NotNodes
Andreas Kohn (JIRA)
jira-events at lists.jboss.org
Tue Apr 20 09:22:49 EDT 2010
[ https://jira.jboss.org/jira/browse/JBRULES-2465?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12526592#action_12526592 ]
Andreas Kohn commented on JBRULES-2465:
---------------------------------------
I went through the commit 32581 and as far as I understand you actually removed the whole idea of RuleRemovalContext keeping track of which nodes are visited. I'm now backporting this change to our drools 5.0.1 version so I can stress it with the same type of queries that exhibited the issue earlier.
I have some questions about the changes:
* drools-core/src/main/java/org/drools/reteoo/Rete.java: doRemove() doesn't remove entry point nodes at all now. I think I have seen similar ClassCastExceptions about entry point nodes, I guess this part will fix those?
* drools-core/src/main/java/org/drools/reteoo/builder/BuildUtils.java: seems to fix an issue with node sharing? Is this a different issue, or required to fix the class cast exception?
* you did not add the assertions I suggested for LeftTupleSource#removeTupleSink() and ObjectSource#removeObjectSource(). Are those dangerous?
Also I noticed a left-over System.out.println("ERROR") in drools-core/src/main/java/org/drools/reteoo/ReteooBuilder.java (IdGenerator#releaseId()). As this is a LinkedList the check whether the id is recycled twice is also quite expensive, so this should maybe be an assert only?
> 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
More information about the jboss-jira
mailing list