[jboss-jira] [JBoss JIRA] (JBRULES-3617) NullpointerException in Accumulate when use query

Mario Fusco (JIRA) jira-events at lists.jboss.org
Mon Oct 1 03:35:03 EDT 2012


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

Mario Fusco commented on JBRULES-3617:
--------------------------------------

The problem arises when you try to concurrently modify a rulebase while another thread is already using the knowledge session. Originally I thought the problem was caused by a race condition, but this is not the case. Edson suggested that it could have been caused by the shared alpha node or anyway by the Rete network being not correctly created, but also this is not completely correct. So let me explain step by step what happens in reality:

1. A new package is added to the knowledge base. That triggers a modification of the rulebase and then a write lock is acquired on it. At the end of this process the Rete network will look as it follows:

[EntryPointNode(1) EntryPoint::DEFAULT ]
    [ObjectTypeNode(2)::EntryPoint::DEFAULT objectType=[ClassObjectType class=org.drools.integrationtests.JbRules3617$Person] expiration=-1ms ]
        [AlphaNode(7) constraint=firstname == "Tom"]
            [AlphaNode(11) constraint=secondname == "Cat"]
                [ AccumulateNode(12) ]
                    [RuleTerminalNode(14): rule=test]
            [JoinNode(8) - [ClassObjectType class=org.drools.integrationtests.JbRules3617$Person]]
                [QueryTerminalNode(9): query=find]

2. In the meanwhile the other thread is trying to update a fact. Since the main thread is modifying the rulebase it will wait acquiring a read lock on it (NamedEntryPoint#389).

3. The Rete builder creates the JoinNode and when it gets attached to the Rete it propagates a sink update on the Alpha and on the ObjectType nodes. This last one triggers an assertObject of the (only one) fact handle in its memory. Since the fact satisfies the constraint of the alpha node (the Person's name is "Tom") the assertObject is propagated to the JoinNode that creates a new RightTuple. This RightTuple (having the JoinNode as sink) is added to the FactHandle itself.

4. The Rete builder creates the AccumulateNode. Following exactly the same path of the former point, when it gets attached to the Rete, a second RightTuple (having the AccumulateNode as sink) is added to the FactHandle. Moreover, the same FactHandle is also accumulated by the AccumulateNode. I am reporting below the source code of the generated accumulator because it will be useful to better explain what happens in the following steps:

public class Accumulate0 implements java.io.Serializable {
   
    private java.util.Map reverseMap = new java.util.HashMap<Integer, ReverseSnapshot>();

    private HashSet set;
   
    public Accumulate0() {  }

    public void init() throws Exception {
        set = new HashSet();
    }
   
    public void accumulate(org.drools.WorkingMemory workingMemory,
                           org.drools.common.InternalFactHandle handle,
                           org.drools.rule.Declaration[] innerDeclarations,
                           Object object,
                           java.lang.String $addr) throws Exception {
       
        this.reverseMap.put( Integer.valueOf( handle.getId() ),
                             new ReverseSnapshot( $addr ) );
        set.add($addr);
    }
   
    public void reverse(org.drools.WorkingMemory workingMemory,
                           org.drools.common.InternalFactHandle handle,
                           Object object ) throws Exception {
        ReverseSnapshot snapshot = (ReverseSnapshot) this.reverseMap.remove( Integer.valueOf( handle.getId() ) );
        java.lang.String $addr = snapshot.$addr;
        set.remove($addr);
    }
   
    public Object getResult( ) throws Exception {
        return ( set );
    }
   
    public boolean supportsReverse() {
        return true;
    }
   
    private static class ReverseSnapshot implements java.io.Serializable {
        public final java.lang.String $addr;
        public ReverseSnapshot( final java.lang.String $addr ) {
            this.$addr = $addr;
        }
    }
}

5. The building process of the Rete network is finished so the main thread has released the write lock on it. The concurrent one can finally acquire the read lock and go forward with the fact update. In the EntryPointNode.modifyObject method a new ModifyPreviousTuples is created (EntryPointNode#260) and here it comes my first doubt: for what said in the former steps the FactHandle now has 2 RightTuples, one having the JoinNode as sink and the other having the AccumulateNode, but for some reason only the first one is carried in the ModifyPreviousTuples object. Is this (always) correct?

6. The object modification hits the AccumulateNode. Here (BetaNode#520) the RightTupleSink in the ModifyPreviousTuples (the JoinNode) is different from the current node (the AccumulateNode), or more in details their rightInputOtnIds are different. This is the main cause of the bug because, for this reason, the RightTuple is NOT removed from the ModifyPreviousTuples and the FactHandle is asserted (again) on the AccumulateNode. It means that the accumulate method of the Accumulator (see code above) is called again with the same FactHandle and a new ReverseSnapshot replaces the existing in the reverseMap.

7. At the end of the EntryPointNode.modifyObject method (EntryPointNode#301), since the ModifyPreviousTuples still has a RightTuple having the AccumulateNode as sink, the AccumulateNode itself retracts the FactHandle by invoking the reverse method of its Accumulator. Here the ReverseSnapshot corresponding to the (only) accumulated fact is removed leaving the reverseMap empty.

8. The infinite loop in the concurrent thread tries to update the same FactHandle again. This time the Rete network isn't changed so the update is correctly propagated calling the modifyRightTuple method of the AccumulateNode. Here the AccumulateNode tries to remove the FactHandle from the Accumulator before to add it again, but when it invokes the reverse method the reverseMap is empty, so the ReverseSnapshot is null and the subsequent statement throws the NPE reported in the jira ticket.
                
> NullpointerException in Accumulate when use query
> -------------------------------------------------
>
>                 Key: JBRULES-3617
>                 URL: https://issues.jboss.org/browse/JBRULES-3617
>             Project: Drools
>          Issue Type: Bug
>      Security Level: Public(Everyone can see) 
>          Components: drools-core  (expert)
>    Affects Versions: 5.4.0.Final
>         Environment: OS: Windows xp
> IDE: Eclipse
> JRE: 1.6
>            Reporter: Yingzhi Wang
>            Assignee: Mark Proctor
>            Priority: Critical
>              Labels: Accumulate, query, reverse
>
> FACT: Person
> package test;
> public class Person {
> 	
> 	private String firstname;
> 	private String secondname;
> 	private String address;
> 	
> 	public Person() {
> 	}
> 	
> 	public String getFirstname() {
> 		return firstname;
> 	}
> 	public void setFirstname(String firstname) {
> 		this.firstname = firstname;
> 	}
> 	public String getSecondname() {
> 		return secondname;
> 	}
> 	public void setSecondname(String secondname) {
> 		this.secondname = secondname;
> 	}
> 	public String getAddress() {
> 		return address;
> 	}
> 	public void setAddress(String address) {
> 		this.address = address;
> 	}
> 	
> }
> r1.drl: 
> package test1
> import test.Person
> import java.util.HashSet
> query find(String $first, String $second)
> 	p: Person(firstname == "111", secondname==$second)
> end
> rule "test"
> 	no-loop true    	
>     when
>     	$list: HashSet( size >= 0 ) 
>     		from accumulate (
>     			Person($addr: address, firstname=="Tom", secondname=="Cat"),
>     			init ( HashSet set = new HashSet(); ),
>     			action ( set.add($addr); ),
>     			reverse ( set.remove($addr); ),
>     			result(set) )
>     then
>     	System.out.println("It' OK~!");
> end
> Test.java
> package test;
> import java.util.Collection;
> import org.drools.KnowledgeBase;
> import org.drools.KnowledgeBaseFactory;
> import org.drools.builder.KnowledgeBuilder;
> import org.drools.builder.KnowledgeBuilderError;
> import org.drools.builder.KnowledgeBuilderErrors;
> import org.drools.builder.KnowledgeBuilderFactory;
> import org.drools.builder.ResourceType;
> import org.drools.definition.KnowledgePackage;
> import org.drools.io.ResourceFactory;
> import org.drools.runtime.StatefulKnowledgeSession;
> import org.drools.runtime.rule.FactHandle;
> public class Test {
> 	public static void main(String[] args) throws Exception {
> 		new Test().test();
> 	}
> 	
> 	public void test() throws Exception {
> 		KnowledgeBase base = KnowledgeBaseFactory.newKnowledgeBase();
> 		StatefulKnowledgeSession s = base.newStatefulKnowledgeSession();
> 		new Th(s).start();
> 		Thread.sleep(2000);
> 		
> 		KnowledgeBuilder builder = KnowledgeBuilderFactory.newKnowledgeBuilder();
> 		builder.add(ResourceFactory.newClassPathResource("r1.drl", getClass()), ResourceType.DRL);
> 		KnowledgeBuilderErrors errs = builder.getErrors();
> 		if (!errs.isEmpty()) {
> 			for (KnowledgeBuilderError e: errs) {
> 				System.out.println(e.toString());
> 			}
> 		}
> 		Collection<KnowledgePackage> col = builder.getKnowledgePackages();
> 		base.addKnowledgePackages(col);
> 	}
> 	
> 	class Th extends Thread {
> 		StatefulKnowledgeSession s;
> 		public Th(StatefulKnowledgeSession s) {
> 			this.s = s;
> 		}
> 		public void run() {
> 			Person p = new Person();
> 			p.setFirstname("Tom");
> 			p.setSecondname("Cat");
> 			p.setAddress("Mars");
> 			FactHandle fact = s.insert(p);
> 			int i=0;
> 			while (true) {
> 				s.update(fact, p);
> 				s.fireAllRules();
> 				try {
> 					Thread.sleep(1000);
> 				} catch (InterruptedException e) {}
> 				i++;
> 			}
> 		}
> 	}
> }
> Run Test.java, will throws Exception: 
> Exception in thread "Thread-0" org.drools.RuntimeDroolsException: java.lang.NullPointerException
> 	at org.drools.rule.Accumulate.reverse(Accumulate.java:212)
> 	at org.drools.reteoo.AccumulateNode.removeMatch(AccumulateNode.java:902)
> 	at org.drools.reteoo.AccumulateNode.modifyRightTuple(AccumulateNode.java:537)
> 	at org.drools.reteoo.BetaNode.modifyObject(BetaNode.java:531)
> 	at org.drools.reteoo.SingleObjectSinkAdapter.propagateModifyObject(SingleObjectSinkAdapter.java:68)
> 	at org.drools.reteoo.AlphaNode.modifyObject(AlphaNode.java:157)
> 	at org.drools.reteoo.SingleObjectSinkAdapter.propagateModifyObject(SingleObjectSinkAdapter.java:68)
> 	at org.drools.reteoo.AlphaNode.modifyObject(AlphaNode.java:157)
> 	at org.drools.reteoo.CompositeObjectSinkAdapter.doPropagateModifyObject(CompositeObjectSinkAdapter.java:507)
> 	at org.drools.reteoo.CompositeObjectSinkAdapter.propagateModifyObject(CompositeObjectSinkAdapter.java:421)
> 	at org.drools.reteoo.ObjectTypeNode.modifyObject(ObjectTypeNode.java:314)
> 	at org.drools.reteoo.EntryPointNode.modifyObject(EntryPointNode.java:265)
> 	at org.drools.common.NamedEntryPoint.update(NamedEntryPoint.java:470)
> 	at org.drools.common.AbstractWorkingMemory.update(AbstractWorkingMemory.java:960)
> 	at org.drools.common.AbstractWorkingMemory.update(AbstractWorkingMemory.java:933)
> 	at org.drools.impl.StatefulKnowledgeSessionImpl.update(StatefulKnowledgeSessionImpl.java:284)
> 	at test.Test$Th.run(Test.java:53)
> Caused by: java.lang.NullPointerException
> 	at test1.Rule_test_b59970efe0844a8d8797758b863454b1$Accumulate0.reverse(Rule_test_b59970efe0844a8d8797758b863454b1.java:52)
> 	at test1.Rule_test_b59970efe0844a8d8797758b863454b1Accumulate0Invoker.reverse(Rule_test_b59970efe0844a8d8797758b863454b1Accumulate0Invoker.java:69)
> 	at org.drools.rule.Accumulate.reverse(Accumulate.java:203)
> 	... 16 more
> But, if I change the query to like this: 
> query find(String $first, String $second)
> 	p: Person(firstname == "111", secondname==$second)
> end
> It will be ok.
> However if I change the query to like this: 
> query find(String $first, String $second)
> 	p: Person(firstname == "Tom", secondname==$second)
> end
> It will throws Exception again.
> firstname=="Tom" is a condition of rule "test"

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira


More information about the jboss-jira mailing list