You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ma...@apache.org on 2009/09/09 15:09:27 UTC

svn commit: r812938 - in /commons/proper/pool/trunk: src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java src/java/org/apache/commons/pool/impl/GenericObjectPool.java xdocs/changes.xml

Author: markt
Date: Wed Sep  9 13:09:26 2009
New Revision: 812938

URL: http://svn.apache.org/viewvc?rev=812938&view=rev
Log:
Fix POOL-149.
Any test that uses latch.getPair() to determine if an object has been allocated, should also check latch.mayCreate()
This corrects two possible failure modes:
- threads waiting indefinitely for objects regardless of whether or not an object is available
- leaks in the internal processing count that could lead to pool exhaustion

Modified:
    commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
    commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
    commons/proper/pool/trunk/xdocs/changes.xml

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=812938&r1=812937&r2=812938&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 Wed Sep  9 13:09:26 2009
@@ -1108,9 +1108,9 @@
                         case WHEN_EXHAUSTED_GROW:
                             // allow new object to be created
                             synchronized (this) {
-                                // Make sure allocate hasn't already assigned an object
-                                // in a different thread
-                                if (latch.getPair() == null) {
+                                // Make sure another thread didn't allocate us an object
+                                // or permit a new object to be created
+                                if (latch.getPair() == null && !latch.mayCreate()) {
                                     _allocationQueue.remove(latch);
                                     latch.getPool().incrementInternalProcessingCount();
                                 }
@@ -1119,8 +1119,8 @@
                         case WHEN_EXHAUSTED_FAIL:
                             synchronized (this) {
                                 // Make sure allocate hasn't already assigned an object
-                                // in a different thread
-                                if (latch.getPair() != null) {
+                                // in a different thread or permitted a new object to be created
+                                if (latch.getPair() != null || latch.mayCreate()) {
                                     break;
                                 }
                                 _allocationQueue.remove(latch);
@@ -1130,7 +1130,8 @@
                             try {
                                 synchronized (latch) {
                                     // Before we wait, make sure another thread didn't allocate us an object
-                                    if (latch.getPair() == null) {
+                                    // or permit a new object to be created
+                                    if (latch.getPair() == null && !latch.mayCreate()) {
                                         if (maxWait <= 0) {
                                             latch.wait();
                                         } else {
@@ -1154,8 +1155,8 @@
                             if (maxWait > 0 && ((System.currentTimeMillis() - starttime) >= maxWait)) {
                                 synchronized (this) {
                                     // Make sure allocate hasn't already assigned an object
-                                    // in a different thread
-                                    if (latch.getPair() == null) {
+                                    // in a different thread or permitted a new object to be created
+                                    if (latch.getPair() == null && !latch.mayCreate()) {
                                         _allocationQueue.remove(latch);
                                     } else {
                                         break;

Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java?rev=812938&r1=812937&r2=812938&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java (original)
+++ commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java Wed Sep  9 13:09:26 2009
@@ -1076,9 +1076,9 @@
                         case WHEN_EXHAUSTED_GROW:
                             // allow new object to be created
                             synchronized (this) {
-                                // Make sure allocate hasn't already assigned an object
-                                // in a different thread
-                                if (latch.getPair() == null) {
+                                // Make sure another thread didn't allocate us an object
+                                // or permit a new object to be created
+                                if (latch.getPair() == null && !latch.mayCreate()) {
                                     _allocationQueue.remove(latch);
                                     _numInternalProcessing++;
                                 }
@@ -1087,8 +1087,8 @@
                         case WHEN_EXHAUSTED_FAIL:
                             synchronized (this) {
                                 // Make sure allocate hasn't already assigned an object
-                                // in a different thread
-                                if (latch.getPair() != null) {
+                                // in a different thread or permitted a new object to be created
+                                if (latch.getPair() != null || latch.mayCreate()) {
                                     break;
                                 }
                                 _allocationQueue.remove(latch);
@@ -1098,7 +1098,8 @@
                             try {
                                 synchronized (latch) {
                                     // Before we wait, make sure another thread didn't allocate us an object
-                                    if (latch.getPair() == null) {
+                                    // or permit a new object to be created
+                                    if (latch.getPair() == null && !latch.mayCreate()) {
                                         if(maxWait <= 0) {
                                             latch.wait();
                                         } else {
@@ -1122,8 +1123,8 @@
                             if(maxWait > 0 && ((System.currentTimeMillis() - starttime) >= maxWait)) {
                                 synchronized(this) {
                                     // Make sure allocate hasn't already assigned an object
-                                    // in a different thread
-                                    if (latch.getPair() == null) {
+                                    // in a different thread or permitted a new object to be created
+                                    if (latch.getPair() == null && !latch.mayCreate()) {
                                         // Remove latch from the allocation queue
                                         _allocationQueue.remove(latch);
                                     } else {

Modified: commons/proper/pool/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/xdocs/changes.xml?rev=812938&r1=812937&r2=812938&view=diff
==============================================================================
--- commons/proper/pool/trunk/xdocs/changes.xml (original)
+++ commons/proper/pool/trunk/xdocs/changes.xml Wed Sep  9 13:09:26 2009
@@ -22,7 +22,14 @@
   </properties>
   <body>
 
-  <release version="1.5.3" date="TBD">
+  <release version="1.5.3" date="TBD" description=
+"This is a patch release containing a fix for POOL-149, a regression
+introduced in version 1.5.">
+    <action dev="markt" type="fix" issue="POOL-149" due-to="Shuyang Zhou">
+      Fix case where a thread could end up waiting indefinitely even if objects
+      were available. Also fixes a couple of leaks in the internal processing
+      object count that could lead to pool exhaustion.
+    </action>
   </release>
   <release version="1.5.2" date="2009-07-12" description=
 "This is a patch release containing fixes for POOL-146 and POOL-147, regressions