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
Index: drools/drools-core/src/main/java/org/drools/reteoo/BetaNode.java
===================================================================
--- drools/drools-core/src/main/java/org/drools/reteoo/BetaNode.java (revision 62882)
+++ drools/drools-core/src/main/java/org/drools/reteoo/BetaNode.java (revision 62883)
@@ -272,17 +272,22 @@
workingMemories[i].clearNodeMemory( this );
}
}
+
+ // check if the left node was visited before walking over the right edge.
+ // it may be that right has leftInput indirectly as source, and even if then right has removed the sink
+ // and marked leftInput as visited we still have leftInput pointing to us (but not we back there!)
+ // XXX: this needs a lot of verification!
+ 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 );
}
-
}
public boolean isObjectMemoryEnabled() {
Index: drools/drools-core/src/main/java/org/drools/reteoo/ObjectSource.java
===================================================================
--- drools/drools-core/src/main/java/org/drools/reteoo/ObjectSource.java (revision 62882)
+++ drools/drools-core/src/main/java/org/drools/reteoo/ObjectSource.java (revision 62883)
@@ -160,6 +160,7 @@
}
if ( this.sink instanceof SingleObjectSinkAdapter ) {
+ assert ((SingleObjectSinkAdapter) sink).sink == objectSink : "Cannot remove non-existing sink";
this.sink = EmptyObjectSinkAdapter.getInstance();
} else {
final CompositeObjectSinkAdapter sinkAdapter = (CompositeObjectSinkAdapter) this.sink;
signature.asc
Description: This is a digitally signed message part
_______________________________________________ rules-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/rules-dev
