[sling-org-apache-sling-commons-threads] 19/27: 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.

[ro...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rombert pushed a commit to annotated tag org.apache.sling.commons.threads-3.2.0
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-threads.git

commit 92432586307eaf7bf31c989a4f74bdd0958fa198
Author: Ian Boston <ie...@apache.org>
AuthorDate: Thu Nov 1 00:22:50 2012 +0000

    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.
    
    git-svn-id: https://svn.apache.org/repos/asf/sling/trunk/bundles/commons/threads@1404430 13f79535-47bb-0310-9956-ffa450edef68
---
 .../threads/impl/DefaultThreadPoolManager.java     | 38 ++++++++++++++--------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPoolManager.java b/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPoolManager.java
index 65f3059..024b2e6 100644
--- a/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPoolManager.java
+++ b/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPoolManager.java
@@ -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();
             }
         }
 

-- 
To stop receiving notification emails like this one, please contact
"commits@sling.apache.org" <co...@sling.apache.org>.