[jboss-svn-commits] JBL Code SVN: r26918 - in labs/jbossrules/branches/5.0.x: drools-compiler/src/test/java/org/drools/integrationtests and 3 other directories.

jboss-svn-commits at lists.jboss.org jboss-svn-commits at lists.jboss.org
Wed Jun 10 22:33:46 EDT 2009


Author: tirelli
Date: 2009-06-10 22:33:45 -0400 (Wed, 10 Jun 2009)
New Revision: 26918

Added:
   labs/jbossrules/branches/5.0.x/drools-compiler/src/test/resources/org/drools/integrationtests/test_MapAccessWithVariable.drl
Modified:
   labs/jbossrules/branches/5.0.x/drools-api/src/main/java/org/drools/runtime/rule/StatelessRuleSession.java
   labs/jbossrules/branches/5.0.x/drools-compiler/src/test/java/org/drools/integrationtests/MiscTest.java
   labs/jbossrules/branches/5.0.x/drools-compiler/src/test/java/org/drools/rule/builder/dialect/mvel/MVELSalienceBuilderTest.java
   labs/jbossrules/branches/5.0.x/drools-core/src/main/java/org/drools/common/AbstractWorkingMemory.java
Log:
JBRULES-2117: fixing concurrent calls to fireXXX() methods. Also, adding test case for MVEL regression on map syntax using variables as keys.

Modified: labs/jbossrules/branches/5.0.x/drools-api/src/main/java/org/drools/runtime/rule/StatelessRuleSession.java
===================================================================
--- labs/jbossrules/branches/5.0.x/drools-api/src/main/java/org/drools/runtime/rule/StatelessRuleSession.java	2009-06-11 02:31:46 UTC (rev 26917)
+++ labs/jbossrules/branches/5.0.x/drools-api/src/main/java/org/drools/runtime/rule/StatelessRuleSession.java	2009-06-11 02:33:45 UTC (rev 26918)
@@ -1,6 +1,5 @@
 package org.drools.runtime.rule;
 
-import org.drools.runtime.StatelessKnowledgeSessionResults;
 
 /**
  * This interface is used as part of the StatelessKnowledSession, which is the interface returned from the KnowledgeBase.

Modified: labs/jbossrules/branches/5.0.x/drools-compiler/src/test/java/org/drools/integrationtests/MiscTest.java
===================================================================
--- labs/jbossrules/branches/5.0.x/drools-compiler/src/test/java/org/drools/integrationtests/MiscTest.java	2009-06-11 02:31:46 UTC (rev 26917)
+++ labs/jbossrules/branches/5.0.x/drools-compiler/src/test/java/org/drools/integrationtests/MiscTest.java	2009-06-11 02:33:45 UTC (rev 26918)
@@ -4633,7 +4633,39 @@
         assertEquals( 1,
                       list.size() );
         assertTrue( list.contains( map ) );
+    }
 
+    // this is an MVEL regression that we need fixed in mvel-2.0.11
+    public void FIXME_testMapAccessWithVariable() throws Exception {
+        final PackageBuilder builder = new PackageBuilder();
+        builder.addPackageFromDrl( new InputStreamReader( getClass().getResourceAsStream( "test_MapAccessWithVariable.drl" ) ) );
+        final Package pkg = builder.getPackage();
+
+        RuleBase ruleBase = getRuleBase();
+        ruleBase.addPackage( pkg );
+        ruleBase = SerializationHelper.serializeObject( ruleBase );
+        final WorkingMemory workingMemory = ruleBase.newStatefulSession();
+
+        final List list = new ArrayList();
+        workingMemory.setGlobal( "results",
+                                 list );
+
+        Map map = new HashMap();
+        map.put( "name",
+                 "Edson" );
+        map.put( "surname",
+                 "Tirelli" );
+        map.put( "age",
+                 "28" );
+
+        workingMemory.insert( map );
+        workingMemory.insert( "name" );
+
+        workingMemory.fireAllRules();
+
+        assertEquals( 1,
+                      list.size() );
+        assertTrue( list.contains( map ) );
     }
 
     public void testHalt() throws Exception {
@@ -6660,6 +6692,50 @@
 
     }
 
+    public void testFireAllWhenFiringUntilHalt() {
+        KnowledgeBase kbase = KnowledgeBaseFactory.newKnowledgeBase();
+        final StatefulKnowledgeSession ksession = kbase.newStatefulKnowledgeSession();
+        
+        Runnable fireUntilHalt = new Runnable() {
+            public void run() {
+                ksession.fireUntilHalt();
+            }
+        };
+        Runnable fireAllRules = new Runnable() {
+            public void run() {
+                ksession.fireAllRules();
+            }
+        };
+        Thread t1 = new Thread( fireUntilHalt );
+        Thread t2 = new Thread( fireAllRules );
+        t1.start();
+        try {
+            Thread.currentThread().sleep( 500 );
+        } catch ( InterruptedException e ) {
+        }
+        t2.start();
+        // give the chance for t2 to finish
+        try {
+            Thread.currentThread().sleep( 1000 );
+        } catch ( InterruptedException e ) {
+        }
+        boolean aliveT2 = t2.isAlive();
+        ksession.halt();
+        try {
+            Thread.currentThread().sleep( 1000 );
+        } catch ( InterruptedException e ) {
+        }
+        boolean aliveT1 = t1.isAlive();
+        if( t2.isAlive() ){
+            t2.interrupt();
+        }
+        if( t1.isAlive() ) {
+            t1.interrupt();
+        }
+        assertFalse( "T2 should have finished", aliveT2 );
+        assertFalse( "T1 should have finished", aliveT1 );
+    }
+    
     public void testDroolsQueryCleanup() {
         KnowledgeBuilder kbuilder = KnowledgeBuilderFactory.newKnowledgeBuilder();
         kbuilder.add( ResourceFactory.newClassPathResource( "test_QueryMemoryLeak.drl",

Modified: labs/jbossrules/branches/5.0.x/drools-compiler/src/test/java/org/drools/rule/builder/dialect/mvel/MVELSalienceBuilderTest.java
===================================================================
--- labs/jbossrules/branches/5.0.x/drools-compiler/src/test/java/org/drools/rule/builder/dialect/mvel/MVELSalienceBuilderTest.java	2009-06-11 02:31:46 UTC (rev 26917)
+++ labs/jbossrules/branches/5.0.x/drools-compiler/src/test/java/org/drools/rule/builder/dialect/mvel/MVELSalienceBuilderTest.java	2009-06-11 02:33:45 UTC (rev 26918)
@@ -29,7 +29,6 @@
     private InstrumentedBuildContent context;
     private RuleBase                 ruleBase;
 
-    @Override
     protected void setUp() throws Exception {
         super.setUp();
         final Package pkg = new Package( "pkg1" );

Copied: labs/jbossrules/branches/5.0.x/drools-compiler/src/test/resources/org/drools/integrationtests/test_MapAccessWithVariable.drl (from rev 26915, labs/jbossrules/trunk/drools-compiler/src/test/resources/org/drools/integrationtests/test_MapAccessWithVariable.drl)
===================================================================
--- labs/jbossrules/branches/5.0.x/drools-compiler/src/test/resources/org/drools/integrationtests/test_MapAccessWithVariable.drl	                        (rev 0)
+++ labs/jbossrules/branches/5.0.x/drools-compiler/src/test/resources/org/drools/integrationtests/test_MapAccessWithVariable.drl	2009-06-11 02:33:45 UTC (rev 26918)
@@ -0,0 +1,13 @@
+package org.drools;
+
+import java.util.Map;
+
+global java.util.List results;
+
+rule "map access with variable"
+	when
+	    $key : String( )
+		$m : Map( this[$key] == "Edson" )
+	then
+		results.add( $m );
+end
\ No newline at end of file

Modified: labs/jbossrules/branches/5.0.x/drools-core/src/main/java/org/drools/common/AbstractWorkingMemory.java
===================================================================
--- labs/jbossrules/branches/5.0.x/drools-core/src/main/java/org/drools/common/AbstractWorkingMemory.java	2009-06-11 02:31:46 UTC (rev 26917)
+++ labs/jbossrules/branches/5.0.x/drools-core/src/main/java/org/drools/common/AbstractWorkingMemory.java	2009-06-11 02:33:45 UTC (rev 26918)
@@ -127,7 +127,7 @@
     // ------------------------------------------------------------
     // Instance members
     // ------------------------------------------------------------
-    protected int                                                            id;
+    protected int                                                             id;
 
     /** The arguments used when adding/removing a property change listener. */
     protected Object[]                                                        addRemovePropertyChangeListenerArgs;
@@ -164,7 +164,7 @@
     protected Queue<WorkingMemoryAction>                                      actionQueue;
 
     protected volatile boolean                                                evaluatingActionQueue;
-    
+
     protected ReentrantLock                                                   lock;
 
     protected boolean                                                         discardOnLogicalOverride;
@@ -215,9 +215,9 @@
     private Map<String, ExitPoint>                                            exitPoints;
 
     private Environment                                                       environment;
-    
-    private ExecutionResults                                              batchExecutionResult;
 
+    private ExecutionResults                                                  batchExecutionResult;
+
     // ------------------------------------------------------------
     // Constructors
     // ------------------------------------------------------------
@@ -257,13 +257,13 @@
         this.ruleBase = ruleBase;
         this.handleFactory = handleFactory;
         this.environment = environment;
-        
-        Globals globals =  ( Globals ) this.environment.get( EnvironmentName.GLOBALS );
+
+        Globals globals = (Globals) this.environment.get( EnvironmentName.GLOBALS );
         if ( globals != null ) {
-            if ( !(globals instanceof GlobalResolver )) {
-                this.globalResolver = new GlobalsAdapter( globals );   
+            if ( !(globals instanceof GlobalResolver) ) {
+                this.globalResolver = new GlobalsAdapter( globals );
             } else {
-                this.globalResolver = ( GlobalResolver ) globals;
+                this.globalResolver = (GlobalResolver) globals;
             }
         } else {
             this.globalResolver = new MapGlobalResolver();
@@ -329,24 +329,26 @@
         initPartitionManagers();
         initTransient();
     }
-    
-    public static class GlobalsAdapter implements GlobalResolver {
+
+    public static class GlobalsAdapter
+        implements
+        GlobalResolver {
         private Globals globals;
-        
+
         public GlobalsAdapter(Globals globals) {
             this.globals = globals;
         }
-        
-        
+
         public Object resolveGlobal(String identifier) {
             return this.globals.get( identifier );
         }
 
         public void setGlobal(String identifier,
                               Object value) {
-            this.globals.set( identifier, value );
+            this.globals.set( identifier,
+                              value );
         }
-        
+
     }
 
     // ------------------------------------------------------------
@@ -367,7 +369,7 @@
 
         for ( EntryPointNode entryPointNode : this.ruleBase.getRete().getEntryPointNodes().values() ) {
             EntryPoint id = entryPointNode.getEntryPoint();
-            if( !  EntryPoint.DEFAULT.equals( id ) ) {
+            if ( !EntryPoint.DEFAULT.equals( id ) ) {
                 WorkingMemoryEntryPoint wmEntryPoint = new NamedEntryPoint( id,
                                                                             entryPointNode,
                                                                             this );
@@ -623,47 +625,50 @@
         this.agenda.halt();
     }
 
-    public synchronized int fireAllRules() throws FactException {
+    public int fireAllRules() throws FactException {
         return fireAllRules( null,
                              -1 );
     }
 
-    public synchronized int fireAllRules(int fireLimit) throws FactException {
+    public int fireAllRules(int fireLimit) throws FactException {
         return fireAllRules( null,
                              fireLimit );
     }
 
-    public synchronized int fireAllRules(final AgendaFilter agendaFilter) throws FactException {
+    public int fireAllRules(final AgendaFilter agendaFilter) throws FactException {
         return fireAllRules( agendaFilter,
                              -1 );
     }
 
-    public synchronized int fireAllRules(final AgendaFilter agendaFilter,
-                                         int fireLimit) throws FactException {
-        // If we're already firing a rule, then it'll pick up
-        // the firing for any other assertObject(..) that get
-        // nested inside, avoiding concurrent-modification
-        // exceptions, depending on code paths of the actions.
-        if ( isSequential() ) {
-            for ( Iterator it = this.liaPropagations.iterator(); it.hasNext(); ) {
-                ((LIANodePropagation) it.next()).doPropagation( this );
-            }
-        }
-
-        // do we need to call this in advance?
-        executeQueuedActions();
-
-        int fireCount = 0;
+    public int fireAllRules(final AgendaFilter agendaFilter,
+                            int fireLimit) throws FactException {
         if ( this.firing.compareAndSet( false,
                                         true ) ) {
             try {
-                fireCount = this.agenda.fireAllRules( agendaFilter,
-                                                      fireLimit );
+                synchronized ( this ) {
+                    // If we're already firing a rule, then it'll pick up
+                    // the firing for any other assertObject(..) that get
+                    // nested inside, avoiding concurrent-modification
+                    // exceptions, depending on code paths of the actions.
+                    if ( isSequential() ) {
+                        for ( Iterator it = this.liaPropagations.iterator(); it.hasNext(); ) {
+                            ((LIANodePropagation) it.next()).doPropagation( this );
+                        }
+                    }
+
+                    // do we need to call this in advance?
+                    executeQueuedActions();
+
+                    int fireCount = 0;
+                    fireCount = this.agenda.fireAllRules( agendaFilter,
+                                                          fireLimit );
+                    return fireCount;
+                }
             } finally {
                 this.firing.set( false );
             }
         }
-        return fireCount;
+        return 0;
     }
 
     /**
@@ -674,7 +679,7 @@
      * @throws IllegalStateException
      *             if this method is called when running in sequential mode
      */
-    public synchronized void fireUntilHalt() {
+    public void fireUntilHalt() {
         fireUntilHalt( null );
     }
 
@@ -689,19 +694,21 @@
      * @throws IllegalStateException
      *             if this method is called when running in sequential mode
      */
-    public synchronized void fireUntilHalt(final AgendaFilter agendaFilter) {
+    public void fireUntilHalt(final AgendaFilter agendaFilter) {
         if ( isSequential() ) {
             throw new IllegalStateException( "fireUntilHalt() can not be called in sequential mode." );
         }
 
-        executeQueuedActions();
-        try {
-            if ( this.firing.compareAndSet( false,
-                                            true ) ) {
-                this.agenda.fireUntilHalt( agendaFilter );
+        if ( this.firing.compareAndSet( false,
+                                        true ) ) {
+            try {
+                synchronized( this ) {
+                    executeQueuedActions();
+                    this.agenda.fireUntilHalt( agendaFilter );
+                }
+            } finally {
+                this.firing.set( false );
             }
-        } finally {
-            this.firing.set( false );
         }
     }
 
@@ -1133,12 +1140,12 @@
                 // can't retract an already retracted handle
                 return;
             }
-            
+
             // the handle might have been disconnected, so reconnect if it has
             if ( factHandle instanceof DisconnectedFactHandle ) {
                 handle = this.objectStore.reconnect( handle );
             }
-            
+
             removePropertyChangeListener( handle );
 
             if ( activation != null ) {
@@ -1214,12 +1221,12 @@
             this.ruleBase.executeQueuedActions();
 
             InternalFactHandle handle = (InternalFactHandle) factHandle;
-            
+
             // the handle might have been disconnected, so reconnect if it has
             if ( factHandle instanceof DisconnectedFactHandle ) {
                 handle = this.objectStore.reconnect( handle );
             }
-            
+
             if ( handle.getId() == -1 ) {
                 // the handle is invalid, most likely already retracted, so
                 // return
@@ -1254,15 +1261,15 @@
                 // the hashCode and equality has changed, so we must update the
                 // EqualityKey
                 EqualityKey key = handle.getEqualityKey();
-                if(key != null){
-                key.removeFactHandle( handle );
+                if ( key != null ) {
+                    key.removeFactHandle( handle );
 
-                // If the equality key is now empty, then remove it
-                if ( key.isEmpty() ) {
-                    this.tms.remove( key );
+                    // If the equality key is now empty, then remove it
+                    if ( key.isEmpty() ) {
+                        this.tms.remove( key );
+                    }
                 }
             }
-            }
         } finally {
             this.lock.unlock();
             this.ruleBase.readUnlock();
@@ -1287,32 +1294,32 @@
             this.ruleBase.executeQueuedActions();
 
             InternalFactHandle handle = (InternalFactHandle) factHandle;
-            
+
             // the handle might have been disconnected, so reconnect if it has
             if ( factHandle instanceof DisconnectedFactHandle ) {
                 handle = this.objectStore.reconnect( handle );
             }
-            
+
             final Object originalObject = handle.getObject();
 
             if ( this.maintainTms ) {
-                if(handle.getEqualityKey() != null ){
-                int status = handle.getEqualityKey().getStatus();
+                if ( handle.getEqualityKey() != null ) {
+                    int status = handle.getEqualityKey().getStatus();
 
-                // now use an existing EqualityKey, if it exists, else create a
-                // new one
-                EqualityKey key = this.tms.get( object );
-                if ( key == null ) {
-                    key = new EqualityKey( handle,
-                                           status );
-                    this.tms.put( key );
-                } else {
-                    key.addFactHandle( handle );
+                    // now use an existing EqualityKey, if it exists, else create a
+                    // new one
+                    EqualityKey key = this.tms.get( object );
+                    if ( key == null ) {
+                        key = new EqualityKey( handle,
+                                               status );
+                        this.tms.put( key );
+                    } else {
+                        key.addFactHandle( handle );
+                    }
+
+                    handle.setEqualityKey( key );
                 }
-
-                handle.setEqualityKey( key );
             }
-            }
 
             this.handleFactory.increaseFactHandleRecency( handle );
 
@@ -1352,13 +1359,17 @@
                 null,
                 null );
     }
-   public void update(final org.drools.runtime.rule.FactHandle factHandle,
+
+    public void update(final org.drools.runtime.rule.FactHandle factHandle,
                        final Object object,
                        final Rule rule,
                        final Activation activation) throws FactException {
 
-       update((org.drools.FactHandle)factHandle, object, rule, activation);
-   }
+        update( (org.drools.FactHandle) factHandle,
+                object,
+                rule,
+                activation );
+    }
 
     /**
      * modify is implemented as half way retract / assert due to the truth
@@ -1374,7 +1385,7 @@
             this.ruleBase.readLock();
             this.lock.lock();
             this.ruleBase.executeQueuedActions();
-            
+
             // the handle might have been disconnected, so reconnect if it has
             if ( factHandle instanceof DisconnectedFactHandle ) {
                 factHandle = this.objectStore.reconnect( factHandle );
@@ -1485,12 +1496,12 @@
                     try {
                         action.execute( this );
                     } catch ( Exception e ) {
-                        if( e instanceof RuntimeDroolsException ) {
+                        if ( e instanceof RuntimeDroolsException ) {
                             // rethrow the exception
-                            throw ((RuntimeDroolsException)e);
+                            throw ((RuntimeDroolsException) e);
                         } else {
-                            System.err.println("************************************************");
-                            System.err.println("Exception caught while executing action: "+action.toString());
+                            System.err.println( "************************************************" );
+                            System.err.println( "Exception caught while executing action: " + action.toString() );
                             e.printStackTrace();
                         }
                     }
@@ -1831,10 +1842,11 @@
         WorkingMemoryEntryPoint wmEntryPoint = this.entryPoints.get( name );
         return wmEntryPoint;
     }
-    
+
     public Collection<WorkingMemoryEntryPoint> getWorkingMemoryEntryPoints() {
         return this.entryPoints.values();
     }
+
     public ObjectTypeConfigurationRegistry getObjectTypeConfigurationRegistry() {
         return this.typeConfReg;
     }
@@ -1858,22 +1870,22 @@
     public PartitionTaskManager getPartitionManager(final RuleBasePartitionId partitionId) {
         return partitionManagers.get( partitionId );
     }
-    
+
     public void startBatchExecution() {
         this.ruleBase.readLock();
         this.lock.lock();
         this.batchExecutionResult = new BatchExecutionResultImpl();
     }
-    
+
     public BatchExecutionResultImpl getExecutionResult() {
-        return ( BatchExecutionResultImpl ) this.batchExecutionResult;
+        return (BatchExecutionResultImpl) this.batchExecutionResult;
     }
-    
+
     public void endBatchExecution() {
         this.batchExecutionResult = null;
         this.lock.unlock();
         this.ruleBase.readUnlock();
-    }    
+    }
 
     // public static class FactHandleInvalidation implements WorkingMemoryAction
     // {




More information about the jboss-svn-commits mailing list