Mailing-List: contact commits-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@commons.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r776085 -
 /commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
Date: Mon, 18 May 2009 20:37:49 -0000
To: commits@commons.apache.org
From: markt@apache.org
Message-Id: <20090518203750.098B92388898@eris.apache.org>

Author: markt
Date: Mon May 18 20:37:49 2009
New Revision: 776085

URL: http://svn.apache.org/viewvc?rev=776085&view=rev
Log:
Fixes for POOL-136. Make syncs consistent.

Modified:
    commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java

Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java?rev=776085&r1=776084&r2=776085&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java (original)
+++ commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java Mon May 18 20:37:49 2009
@@ -946,7 +946,16 @@
     public Object borrowObject(Object key) throws Exception {
         long starttime = System.currentTimeMillis();
         Latch latch = new Latch(key);
+        byte whenExhaustedAction;
+        long maxWait;
         synchronized (this) {
+            // Get local copy of current config. Can't sync when used later as
+            // it can result in a deadlock. Has the added advantage that config
+            // is consistent for entire method execution 
+            whenExhaustedAction = _whenExhaustedAction;
+            maxWait = _maxWait;
+            
+            // Add this request to the queue 
             _allocationQueue.add(latch);
             allocate();
         }
@@ -962,7 +971,7 @@
                     // allow new object to be created
                 } else {
                     // the pool is exhausted
-                    switch(_whenExhaustedAction) {
+                    switch(whenExhaustedAction) {
                         case WHEN_EXHAUSTED_GROW:
                         // allow new object to be created
                         break;
@@ -974,13 +983,13 @@
                         case WHEN_EXHAUSTED_BLOCK:
                             try {
                                 synchronized (latch) {
-                                    if(_maxWait <= 0) {
+                                    if(maxWait <= 0) {
                                         latch.wait();
                                     } else {
                                         // this code may be executed again after a notify then continue cycle
                                         // so, need to calculate the amount of time to wait
                                         final long elapsed = (System.currentTimeMillis() - starttime);
-                                        final long waitTime = _maxWait - elapsed;
+                                        final long waitTime = maxWait - elapsed;
                                         if (waitTime > 0)
                                         {
                                             latch.wait(waitTime);
@@ -991,13 +1000,13 @@
                                 Thread.currentThread().interrupt();
                                 throw e;
                                 }
-                            if(_maxWait > 0 && ((System.currentTimeMillis() - starttime) >= _maxWait)) {
+                            if(maxWait > 0 && ((System.currentTimeMillis() - starttime) >= maxWait)) {
                                 throw new NoSuchElementException("Timeout waiting for idle object");
                             } else {
                                 continue; // keep looping
                             }
                         default:
-                            throw new IllegalArgumentException("whenExhaustedAction " + _whenExhaustedAction + " not recognized.");
+                            throw new IllegalArgumentException("whenExhaustedAction " + whenExhaustedAction + " not recognized.");
                     }
                 }
             }
@@ -1501,9 +1510,18 @@
      * @throws Exception when there is a problem evicting idle objects.
      */
     public void evict() throws Exception {
-        // Initialize key to last key value
         Object key = null;
+        boolean testWhileIdle;
+        long minEvictableIdleTimeMillis;
+        
         synchronized (this) {
+            // Get local copy of current config. Can't sync when used later as
+            // it can result in a deadlock. Has the added advantage that config
+            // is consistent for entire method execution 
+            testWhileIdle = _testWhileIdle;
+            minEvictableIdleTimeMillis = _minEvictableIdleTimeMillis;
+            
+            // Initialize key to last key value
             if (_evictionKeyCursor != null && 
                     _evictionKeyCursor._lastReturned != null) {
                 key = _evictionKeyCursor._lastReturned.value();
@@ -1582,12 +1600,12 @@
             }
 
             boolean removeObject=false;
-            if((_minEvictableIdleTimeMillis > 0) &&
+            if((minEvictableIdleTimeMillis > 0) &&
                (System.currentTimeMillis() - pair.tstamp > 
-               _minEvictableIdleTimeMillis)) {
+               minEvictableIdleTimeMillis)) {
                 removeObject=true;
             }
-            if(_testWhileIdle && removeObject == false) {
+            if(testWhileIdle && removeObject == false) {
                 boolean active = false;
                 try {
                     _factory.activateObject(key,pair.value);
@@ -1778,7 +1796,7 @@
         return buf.toString();
     }
 
-    private int getNumTests() {
+    private synchronized int getNumTests() {
         if(_numTestsPerEvictionRun >= 0) {
             return _numTestsPerEvictionRun;
         } else {
@@ -1837,27 +1855,35 @@
         private int internalProcessingCount = 0;
 
         void incrementActiveCount() {
-            _totalActive++;
+            synchronized (GenericKeyedObjectPool.this) {
+                _totalActive++;
+            }
             activeCount++;
         }
 
         void decrementActiveCount() {
-            _totalActive--;
+            synchronized (GenericKeyedObjectPool.this) {
+                _totalActive--;
+            }
             if (activeCount > 0) {
                 activeCount--;
             }
         }
 
         void incrementInternalProcessingCount() {
-            _totalInternalProcessing++;
+            synchronized (GenericKeyedObjectPool.this) {
+                _totalInternalProcessing++;
+            }
             internalProcessingCount++;
         }
 
         void decrementInternalProcessingCount() {
-            _totalInternalProcessing--;
+            synchronized (GenericKeyedObjectPool.this) {
+                _totalInternalProcessing--;
+            }
             internalProcessingCount--;
         }
-}
+    }
     
     /**
      * A simple "struct" encapsulating an object instance and a timestamp.