incubator-sling-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From i..@apache.org
Subject svn commit: r1404430 - /sling/trunk/bundles/commons/threads/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPoolManager.java
Date Thu, 01 Nov 2012 00:22:51 GMT
Author: ieb
Date: Thu Nov  1 00:22:50 2012
New Revision: 1404430

URL: http://svn.apache.org/viewvc?rev=1404430&view=rev
Log:
SLING-2535 Undid my previous commit and put the synchronisation one layer further out as orriginally
suggested. Synchronising in incUsage and decUsage is too risky from a deadlock point of view.
Added Javadoc to warn about thread safety on the methods.

Modified:
    sling/trunk/bundles/commons/threads/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPoolManager.java

Modified: sling/trunk/bundles/commons/threads/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPoolManager.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/threads/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPoolManager.java?rev=1404430&r1=1404429&r2=1404430&view=diff
==============================================================================
--- sling/trunk/bundles/commons/threads/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPoolManager.java
(original)
+++ sling/trunk/bundles/commons/threads/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPoolManager.java
Thu Nov  1 00:22:50 2012
@@ -126,6 +126,7 @@ public class DefaultThreadPoolManager
         final String poolName = (name == null ? DEFAULT_THREADPOOL_NAME : name);
         Entry entry = null;
         boolean created = false;
+        ThreadPool threadPool = null;
         synchronized (this.pools) {
             entry = this.pools.get(poolName);
             if ( entry == null ) {
@@ -136,11 +137,12 @@ public class DefaultThreadPoolManager
 
                 this.pools.put(poolName, entry);
             }
+            threadPool = entry.incUsage();
         }
         if (created) {
             entry.registerMBean();
         }
-        return entry.incUsage();
+        return threadPool;
     }
 
     /**
@@ -193,11 +195,13 @@ public class DefaultThreadPoolManager
         final String name = "ThreadPool-" + UUID.randomUUID().toString() +
              (label == null ? "" : " (" + label + ")");
         final Entry entry = new Entry(null, config, name, bundleContext);
+        ThreadPool threadPool = null;
         synchronized ( this.pools ) {
             this.pools.put(name, entry);
+            threadPool = entry.incUsage();
         }
         entry.registerMBean();
-        return entry.incUsage();
+        return threadPool;
     }
 
     /**
@@ -342,22 +346,30 @@ public class DefaultThreadPoolManager
             }
         }
 
+        /**
+         * Increments a usage counter and gets the ThreadPoolFacade inside the Entry.
+         * Note this method is not thread safe and must at all time be protected from
+         * multiple threads accessing it at the same time. The counter is volatile and 
+         * hence not atomic in updates.
+         * @return the thread pool Facade instance after reference counting the usage.
+         */
         public ThreadPoolFacade incUsage() {
-            synchronized (usagelock) {
-                if ( pool == null ) {
-                    pool = new ThreadPoolFacade(new DefaultThreadPool(name, this.config));
-                }
-                this.count++;
-                return pool;
+            if ( pool == null ) {
+                pool = new ThreadPoolFacade(new DefaultThreadPool(name, this.config));
             }
+            this.count++;
+            return pool;
         }
 
+        /**
+         * Decrement the usage counter, and if its zero initiate a shutdown the entry (and
the thread pool). 
+         * Note: this method is not thread safe and must at all time be protected from multiple
threads
+         * accessing it at the same time. The counter is volatile and hence not atomic in
updates.
+         */
         public void decUsage() {
-            synchronized (usagelock) {
-                this.count--;
-                if ( this.count == 0 ) {
-                    this.shutdown();
-                }
+            this.count--;
+            if ( this.count == 0 ) {
+                this.shutdown();
             }
         }
 



Mime
View raw message