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

[Edson Tirelli (JIRA)]

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

Edson Tirelli commented on JBRULES-2465:
----------------------------------------

Andreas,

I am looking into this, but not able to reproduce it yet. I started with this testcase:

========= DRL ==================
package org.drools;
 
rule "subnetworks with not"
	when
		not( 
		    not( Person( name == "bob") ) and 
		    not( Person( name == "mark" ) ) 
		) 
	then
	    // do something
end

========= JUNIT ================
    public void testDynamicRuleRemovalsSubNetworkAndNot() throws Exception {
        final KnowledgeBuilder kbuilder = KnowledgeBuilderFactory.newKnowledgeBuilder();
        kbuilder.add( ResourceFactory.newInputStreamResource( getClass().getResourceAsStream( "test_DynamicRulesWithNotSubnetwork.drl" ) ),
                      ResourceType.DRL );
        assertFalse( kbuilder.getErrors().toString(),
                     kbuilder.hasErrors() );

        final KnowledgeBase kbase = KnowledgeBaseFactory.newKnowledgeBase();
        kbase.addKnowledgePackages( kbuilder.getKnowledgePackages() );

        final StatefulKnowledgeSession ksession = kbase.newStatefulKnowledgeSession();

        final AgendaEventListener alistener = mock( AgendaEventListener.class );
        ksession.addEventListener( alistener );

        // pattern does not match, so do not activate
        ksession.insert( new Person( "toni" ) );
        verify( alistener,
                never() ).activationCreated( any( org.drools.event.rule.ActivationCreatedEvent.class ) );

        // pattern matches, so create activation
        ksession.insert( new Person( "bob" ) );
        verify( alistener,
                times( 1 ) ).activationCreated( any( org.drools.event.rule.ActivationCreatedEvent.class ) );

        // already active, so no new activation should be created
        ksession.insert( new Person( "mark" ) );
        verify( alistener,
                times( 1 ) ).activationCreated( any( org.drools.event.rule.ActivationCreatedEvent.class ) );

        kbase.removeKnowledgePackage( "org.drools" );

        assertEquals( 0,
                      kbase.getKnowledgePackages().size() );

    }
================================

It is working fine. Can you provide more info on how to replicate? If you can show me some of your rules that cause the problem, I might be able to figure what is happening. If you can disclose your rules, but would prefer to do it privately, you can contact me by e-mail "etirelli AT redhat DOT com" or "ed.tirelli AT gmail DOT com".


> 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