You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by rd...@apache.org on 2005/10/27 00:07:14 UTC

svn commit: r328751 - /jakarta/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java

Author: rdonkin
Date: Wed Oct 26 15:07:10 2005
New Revision: 328751

URL: http://svn.apache.org/viewcvs?rev=328751&view=rev
Log:
Added missed synchronization to GenericObjectPool. Submitted by Sandy McArthur. Issue #37227. Thanks to Mayur Naik for discovering and reporting these issues.

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

Modified: jakarta/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java?rev=328751&r1=328750&r2=328751&view=diff
==============================================================================
--- jakarta/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java (original)
+++ jakarta/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java Wed Oct 26 15:07:10 2005
@@ -766,59 +766,56 @@
 
     //-- ObjectPool methods ------------------------------------------
 
-    public Object borrowObject() throws Exception {
+    public synchronized Object borrowObject() throws Exception {
+        assertOpen();
         long starttime = System.currentTimeMillis();
         boolean newlyCreated = false;
         for(;;) {
             ObjectTimestampPair pair = null;
 
-            synchronized(this) {
-                assertOpen();
-
-                // if there are any sleeping, just grab one of those
-                try {
-                    pair = (ObjectTimestampPair)(_pool.removeFirst());
-                } catch(NoSuchElementException e) {
-                    ; /* ignored */
-                }
+            // if there are any sleeping, just grab one of those
+            try {
+                pair = (ObjectTimestampPair)(_pool.removeFirst());
+            } catch(NoSuchElementException e) {
+                ; /* ignored */
+            }
 
-                // otherwise
-                if(null == pair) {
-                    // check if we can create one
-                    // (note we know that the num sleeping is 0, else we wouldn't be here)
-                    if(_maxActive < 0 || _numActive < _maxActive) {
-                        // allow new object to be created
-                    } else {
-                        // the pool is exhausted
-                        switch(_whenExhaustedAction) {
-                            case WHEN_EXHAUSTED_GROW:
-                                // allow new object to be created
-                                break;
-                            case WHEN_EXHAUSTED_FAIL:
-                                throw new NoSuchElementException("Pool exhausted");
-                            case WHEN_EXHAUSTED_BLOCK:
-                                    try {
-                                        if(_maxWait <= 0) {
-                                            wait();
-                                        } else {
-                                            wait(_maxWait);
-                                        }
-                                    } catch(InterruptedException e) {
-                                        // ignored
-                                    }
-                                    if(_maxWait > 0 && ((System.currentTimeMillis() - starttime) >= _maxWait)) {
-                                        throw new NoSuchElementException("Timeout waiting for idle object");
-                                    } else {
-                                        continue; // keep looping
-                                    }
-                            default:
-                                throw new IllegalArgumentException("WhenExhaustedAction property " + _whenExhaustedAction + " not recognized.");
-                        }
+            // otherwise
+            if(null == pair) {
+                // check if we can create one
+                // (note we know that the num sleeping is 0, else we wouldn't be here)
+                if(_maxActive < 0 || _numActive < _maxActive) {
+                    // allow new object to be created
+                } else {
+                    // the pool is exhausted
+                    switch(_whenExhaustedAction) {
+                        case WHEN_EXHAUSTED_GROW:
+                            // allow new object to be created
+                            break;
+                        case WHEN_EXHAUSTED_FAIL:
+                            throw new NoSuchElementException("Pool exhausted");
+                        case WHEN_EXHAUSTED_BLOCK:
+                            try {
+                                if(_maxWait <= 0) {
+                                    wait();
+                                } else {
+                                    wait(_maxWait);
+                                }
+                            } catch(InterruptedException e) {
+                                // ignored
+                            }
+                            if(_maxWait > 0 && ((System.currentTimeMillis() - starttime) >= _maxWait)) {
+                                throw new NoSuchElementException("Timeout waiting for idle object");
+                            } else {
+                                continue; // keep looping
+                            }
+                        default:
+                            throw new IllegalArgumentException("WhenExhaustedAction property " + _whenExhaustedAction + " not recognized.");
                     }
                 }
-                _numActive++;
-            } // end synchronized
-            
+            }
+            _numActive++;
+
             // create new object when needed
             if(null == pair) {
                 try {
@@ -828,10 +825,8 @@
                 }
                 catch (Throwable e) {
                     // object cannot be created
-                    synchronized(this) {
-                        _numActive--;
-                        notifyAll();
-                    }
+                    _numActive--;
+                    notifyAll();
                     if (e instanceof Exception) {
                         throw (Exception) e;
                     } else if (e instanceof Error) {
@@ -852,10 +847,8 @@
             } 
             catch (Throwable e) {
                 // object cannot be activated or is invalid
-                synchronized(this) {
-                    _numActive--;
-                    notifyAll();
-                }
+                _numActive--;
+                notifyAll();
                 try {
                     _factory.destroyObject(pair.value);
                 } 
@@ -872,16 +865,14 @@
         }
     }
 
-    public void invalidateObject(Object obj) throws Exception {
+    public synchronized void invalidateObject(Object obj) throws Exception {
         assertOpen();
         try {
             _factory.destroyObject(obj);
         }
         finally {
-            synchronized(this) {
-                _numActive--;
-                notifyAll(); // _numActive has changed
-            }
+            _numActive--;
+            notifyAll(); // _numActive has changed
         }
     }
 
@@ -909,7 +900,7 @@
         return _pool.size();
     }
 
-    public void returnObject(Object obj) throws Exception {
+    public synchronized void returnObject(Object obj) throws Exception {
         assertOpen();
         addObjectToPool(obj, true);
     }
@@ -928,17 +919,15 @@
 
         boolean shouldDestroy = !success;
 
-        synchronized(this) {
-            if (decrementNumActive) {
-                _numActive--;
-            }
-            if((_maxIdle >= 0) && (_pool.size() >= _maxIdle)) {
-                shouldDestroy = true;
-            } else if(success) {
-                _pool.addFirst(new ObjectTimestampPair(obj));
-            }
-            notifyAll(); // _numActive has changed
+        if (decrementNumActive) {
+            _numActive--;
         }
+        if((_maxIdle >= 0) && (_pool.size() >= _maxIdle)) {
+            shouldDestroy = true;
+        } else if(success) {
+            _pool.addFirst(new ObjectTimestampPair(obj));
+        }
+        notifyAll(); // _numActive has changed
 
         if(shouldDestroy) {
             try {
@@ -1057,7 +1046,8 @@
      * Create an object, and place it into the pool.
      * addObject() is useful for "pre-loading" a pool with idle objects.
      */
-    public void addObject() throws Exception {
+    public synchronized void addObject() throws Exception {
+        assertOpen();
         Object obj = _factory.makeObject();
         addObjectToPool(obj, false);
     }
@@ -1202,7 +1192,7 @@
     * @see #setMinIdle
     * @see #getMinIdle
     */
-   private int _minIdle = DEFAULT_MIN_IDLE;
+    private int _minIdle = DEFAULT_MIN_IDLE;
 
     /**
      * The cap on the total number of active instances from the pool.



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org