[jboss-svn-commits] JBL Code SVN: r9594 - labs/jbossrules/branches/3.0.x/drools-core/src/main/java/org/drools/common.

jboss-svn-commits at lists.jboss.org jboss-svn-commits at lists.jboss.org
Mon Feb 19 08:28:32 EST 2007


Author: tirelli
Date: 2007-02-19 08:28:32 -0500 (Mon, 19 Feb 2007)
New Revision: 9594

Modified:
   labs/jbossrules/branches/3.0.x/drools-core/src/main/java/org/drools/common/AbstractRuleBase.java
Log:
JBRULES-603: wrapping lock/unlock calls into a try/finally block to avoid 'eternal' locks

Modified: labs/jbossrules/branches/3.0.x/drools-core/src/main/java/org/drools/common/AbstractRuleBase.java
===================================================================
--- labs/jbossrules/branches/3.0.x/drools-core/src/main/java/org/drools/common/AbstractRuleBase.java	2007-02-19 13:05:32 UTC (rev 9593)
+++ labs/jbossrules/branches/3.0.x/drools-core/src/main/java/org/drools/common/AbstractRuleBase.java	2007-02-19 13:28:32 UTC (rev 9594)
@@ -221,50 +221,60 @@
         newPkg.checkValidity();
         final Package pkg = (Package) this.pkgs.get( newPkg.getName() );
 
-        // Iterate each workingMemory and lock it
-        // This is so we don't update the Rete network during propagation
-        for ( final Iterator it = this.workingMemories.keySet().iterator(); it.hasNext(); ) {
-            final AbstractWorkingMemory workingMemory = (AbstractWorkingMemory) it.next();
-            workingMemory.getLock().lock();
-        }
+        // INVARIANT: lastAquiredLock always contains the index of the last aquired lock +1 
+        // in the working memory array 
+        int lastAquiredLock = 0;
+        // get a snapshot of current working memories for locking
+        AbstractWorkingMemory[] wms = (AbstractWorkingMemory[]) this.workingMemories.keySet().toArray( new AbstractWorkingMemory[this.workingMemories.size()] );
 
-        if ( pkg != null ) {
-            mergePackage( pkg,
-                          newPkg );
-        } else {
-            this.pkgs.put( newPkg.getName(),
-                           newPkg );
-        }
+        try {
+            // Iterate each workingMemory and lock it
+            // This is so we don't update the Rete network during propagation
+            for ( lastAquiredLock = 0; lastAquiredLock < wms.length; lastAquiredLock++ ) {
+                wms[lastAquiredLock].getLock().lock();
+            }
 
-        final Map newGlobals = newPkg.getGlobals();
+            if ( pkg != null ) {
+                mergePackage( pkg,
+                              newPkg );
+            } else {
+                this.pkgs.put( newPkg.getName(),
+                               newPkg );
+            }
 
-        // Check that the global data is valid, we cannot change the type
-        // of an already declared global variable
-        for ( final Iterator it = newGlobals.keySet().iterator(); it.hasNext(); ) {
-            final String identifier = (String) it.next();
-            final Class type = (Class) newGlobals.get( identifier );
-            if ( this.globals.containsKey( identifier ) && !this.globals.get( identifier ).equals( type ) ) {
-                throw new PackageIntegrationException( pkg );
+            final Map newGlobals = newPkg.getGlobals();
+
+            // Check that the global data is valid, we cannot change the type
+            // of an already declared global variable
+            for ( final Iterator it = newGlobals.keySet().iterator(); it.hasNext(); ) {
+                final String identifier = (String) it.next();
+                final Class type = (Class) newGlobals.get( identifier );
+                if ( this.globals.containsKey( identifier ) && !this.globals.get( identifier ).equals( type ) ) {
+                    throw new PackageIntegrationException( pkg );
+                }
             }
-        }
-        this.globals.putAll( newGlobals );
+            this.globals.putAll( newGlobals );
 
-        final Rule[] rules = newPkg.getRules();
+            final Rule[] rules = newPkg.getRules();
 
-        for ( int i = 0; i < rules.length; ++i ) {
-            addRule( rules[i] );
-        }
+            for ( int i = 0; i < rules.length; ++i ) {
+                addRule( rules[i] );
+            }
 
-        this.packageClassLoader.addClassLoader( newPkg.getPackageCompilationData().getClassLoader() );
+            this.packageClassLoader.addClassLoader( newPkg.getPackageCompilationData().getClassLoader() );
 
-        // Iterate each workingMemory and attempt to fire any rules, that were activated as a result 
-        // of the new rule addition. Unlock after fireAllRules();
-        for ( final Iterator it = this.workingMemories.keySet().iterator(); it.hasNext(); ) {
-            final AbstractWorkingMemory workingMemory = (AbstractWorkingMemory) it.next();
+        } finally {
+            
+            // Iterate each workingMemory and attempt to fire any rules, that were activated as a result 
+            // of the new rule addition. Unlock after fireAllRules();
+            
+            // as per the INVARIANT defined above, we need to iterate from lastAquiredLock-1 to 0. 
+            for ( lastAquiredLock--; lastAquiredLock > -1; lastAquiredLock-- ) {
+                wms[lastAquiredLock].fireAllRules();
+                wms[lastAquiredLock].getLock().unlock();
+            }
+        }
 
-            workingMemory.fireAllRules();
-            workingMemory.getLock().unlock();
-        }
     }
 
     /**
@@ -325,66 +335,95 @@
 
     public void removePackage(final String packageName) {
         final Package pkg = (Package) this.pkgs.get( packageName );
-        // Iterate each workingMemory and lock it
-        // This is so we don't update the Rete network during propagation
-        for ( final Iterator it = this.workingMemories.keySet().iterator(); it.hasNext(); ) {
-            final AbstractWorkingMemory workingMemory = (AbstractWorkingMemory) it.next();
-            workingMemory.getLock().lock();
-        }
 
-        final Rule[] rules = pkg.getRules();
+        // INVARIANT: lastAquiredLock always contains the index of the last aquired lock +1 
+        // in the working memory array 
+        int lastAquiredLock = 0;
+        // get a snapshot of current working memories for locking
+        AbstractWorkingMemory[] wms = (AbstractWorkingMemory[]) this.workingMemories.keySet().toArray( new AbstractWorkingMemory[this.workingMemories.size()] );
 
-        for ( int i = 0; i < rules.length; ++i ) {
-            removeRule( rules[i] );
-        }
+        try {
+            // Iterate each workingMemory and lock it
+            // This is so we don't update the Rete network during propagation
+            for ( lastAquiredLock = 0; lastAquiredLock < wms.length; lastAquiredLock++ ) {
+                wms[lastAquiredLock].getLock().lock();
+            }
 
-        this.packageClassLoader.removeClassLoader( pkg.getPackageCompilationData().getClassLoader() );
+            final Rule[] rules = pkg.getRules();
 
-        pkg.clear();
+            for ( int i = 0; i < rules.length; ++i ) {
+                removeRule( rules[i] );
+            }
 
-        // getting the list of referenced globals 
-        final Set referencedGlobals = new HashSet();
-        for ( final Iterator it = this.pkgs.values().iterator(); it.hasNext(); ) {
-            final org.drools.rule.Package pkgref = (org.drools.rule.Package) it.next();
-            if ( pkgref != pkg ) {
-                referencedGlobals.addAll( pkgref.getGlobals().keySet() );
+            this.packageClassLoader.removeClassLoader( pkg.getPackageCompilationData().getClassLoader() );
+
+            pkg.clear();
+
+            // getting the list of referenced globals 
+            final Set referencedGlobals = new HashSet();
+            for ( final Iterator it = this.pkgs.values().iterator(); it.hasNext(); ) {
+                final org.drools.rule.Package pkgref = (org.drools.rule.Package) it.next();
+                if ( pkgref != pkg ) {
+                    referencedGlobals.addAll( pkgref.getGlobals().keySet() );
+                }
             }
-        }
-        // removing globals declared inside the package that are not shared
-        for ( final Iterator it = pkg.getGlobals().keySet().iterator(); it.hasNext(); ) {
-            final String globalName = (String) it.next();
-            if ( !referencedGlobals.contains( globalName ) ) {
-                this.globals.remove( globalName );
+            // removing globals declared inside the package that are not shared
+            for ( final Iterator it = pkg.getGlobals().keySet().iterator(); it.hasNext(); ) {
+                final String globalName = (String) it.next();
+                if ( !referencedGlobals.contains( globalName ) ) {
+                    this.globals.remove( globalName );
+                }
             }
+            // removing the package itself from the list
+            this.pkgs.remove( pkg.getName() );
+
+        } finally {
+            
+            // Iterate each workingMemory and attempt to fire any rules, that were activated as a result 
+            // of the new rule addition. Unlock after fireAllRules();
+            
+            // as per the INVARIANT defined above, we need to iterate from lastAquiredLock-1 to 0. 
+            for ( lastAquiredLock--; lastAquiredLock > -1; lastAquiredLock-- ) {
+                wms[lastAquiredLock].fireAllRules();
+                wms[lastAquiredLock].getLock().unlock();
+            }
         }
-        // removing the package itself from the list
-        this.pkgs.remove( pkg.getName() );
 
-        // Iterate and unlock
-        for ( final Iterator it = this.workingMemories.keySet().iterator(); it.hasNext(); ) {
-            final AbstractWorkingMemory workingMemory = (AbstractWorkingMemory) it.next();
-            workingMemory.getLock().unlock();
-        }
     }
 
     public void removeRule(final String packageName,
                            final String ruleName) {
         final Package pkg = (Package) this.pkgs.get( packageName );
         final Rule rule = pkg.getRule( ruleName );
-        // Iterate each workingMemory and lock it
-        // This is so we don't update the Rete network during propagation
-        for ( final Iterator it = this.workingMemories.keySet().iterator(); it.hasNext(); ) {
-            final AbstractWorkingMemory workingMemory = (AbstractWorkingMemory) it.next();
-            workingMemory.getLock().lock();
+
+        // INVARIANT: lastAquiredLock always contains the index of the last aquired lock +1 
+        // in the working memory array 
+        int lastAquiredLock = 0;
+        // get a snapshot of current working memories for locking
+        AbstractWorkingMemory[] wms = (AbstractWorkingMemory[]) this.workingMemories.keySet().toArray( new AbstractWorkingMemory[this.workingMemories.size()] );
+
+        try {
+            // Iterate each workingMemory and lock it
+            // This is so we don't update the Rete network during propagation
+            for ( lastAquiredLock = 0; lastAquiredLock < wms.length; lastAquiredLock++ ) {
+                wms[lastAquiredLock].getLock().lock();
+            }
+
+            removeRule( rule );
+            pkg.removeRule( rule );
+
+        } finally {
+            
+            // Iterate each workingMemory and attempt to fire any rules, that were activated as a result 
+            // of the new rule addition. Unlock after fireAllRules();
+            
+            // as per the INVARIANT defined above, we need to iterate from lastAquiredLock-1 to 0. 
+            for ( lastAquiredLock--; lastAquiredLock > -1; lastAquiredLock-- ) {
+                wms[lastAquiredLock].fireAllRules();
+                wms[lastAquiredLock].getLock().unlock();
+            }
         }
-        removeRule( rule );
-        pkg.removeRule( rule );
 
-        // Iterate and unlock
-        for ( final Iterator it = this.workingMemories.keySet().iterator(); it.hasNext(); ) {
-            final AbstractWorkingMemory workingMemory = (AbstractWorkingMemory) it.next();
-            workingMemory.getLock().unlock();
-        }
     }
 
     protected abstract void removeRule(Rule rule);




More information about the jboss-svn-commits mailing list