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 2014/09/04 11:00:24 UTC

svn commit: r1622424 - in /commons/proper/pool/trunk/src: changes/ main/java/org/apache/commons/pool2/impl/ test/java/org/apache/commons/pool2/impl/

Author: markt
Date: Thu Sep  4 09:00:24 2014
New Revision: 1622424

URL: http://svn.apache.org/r1622424
Log:
Fix POOL-276
Ensure that objects are not validated on borrow when testOnBorrow is set to false, testOnCreate is set to true and the pool is exhausted at the point borrowObject() is called.

Modified:
    commons/proper/pool/trunk/src/changes/changes.xml
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
    commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
    commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java

Modified: commons/proper/pool/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/changes/changes.xml?rev=1622424&r1=1622423&r2=1622424&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/changes/changes.xml (original)
+++ commons/proper/pool/trunk/src/changes/changes.xml Thu Sep  4 09:00:24 2014
@@ -44,6 +44,11 @@ The <action> type attribute can be add,u
   </properties>
   <body>
   <release version="2.3" date="TBD" description="TBD">
+    <action dev="markt" type="fix" issue="POOL-276">
+      Ensure that objects are not validated on borrow when testOnBorrow is set
+      to false, testOnCreate is set to true and the pool is exhausted at the
+      point borrowObject() is called.
+    </action>
     <action dev="psteitz" type="fix" issue="POOL-270" due-to="Michael Berman">
       Fixed error in GenericKeyedObjectPool constructor causing minEvictableIdleTimeMillis
       to be used in place of timeBetweenEvictionRunsMillis in eviction timer setup

Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java?rev=1622424&r1=1622423&r2=1622424&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java (original)
+++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java Thu Sep  4 09:00:24 2014
@@ -353,8 +353,10 @@ public class GenericKeyedObjectPool<K,T>
                 if (blockWhenExhausted) {
                     p = objectDeque.getIdleObjects().pollFirst();
                     if (p == null) {
-                        create = true;
                         p = create(key);
+                        if (p != null) {
+                            create = true;
+                        }
                     }
                     if (p == null) {
                         if (borrowMaxWaitMillis < 0) {
@@ -374,8 +376,10 @@ public class GenericKeyedObjectPool<K,T>
                 } else {
                     p = objectDeque.getIdleObjects().pollFirst();
                     if (p == null) {
-                        create = true;
                         p = create(key);
+                        if (p != null) {
+                            create = true;
+                        }
                     }
                     if (p == null) {
                         throw new NoSuchElementException("Pool exhausted");

Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java?rev=1622424&r1=1622423&r2=1622424&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java (original)
+++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java Thu Sep  4 09:00:24 2014
@@ -432,8 +432,10 @@ public class GenericObjectPool<T> extend
             if (blockWhenExhausted) {
                 p = idleObjects.pollFirst();
                 if (p == null) {
-                    create = true;
                     p = create();
+                    if (p != null) {
+                        create = true;
+                    }
                 }
                 if (p == null) {
                     if (borrowMaxWaitMillis < 0) {
@@ -453,8 +455,10 @@ public class GenericObjectPool<T> extend
             } else {
                 p = idleObjects.pollFirst();
                 if (p == null) {
-                    create = true;
                     p = create();
+                    if (p != null) {
+                        create = true;
+                    }
                 }
                 if (p == null) {
                     throw new NoSuchElementException("Pool exhausted");

Modified: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java?rev=1622424&r1=1622423&r2=1622424&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java (original)
+++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java Thu Sep  4 09:00:24 2014
@@ -32,6 +32,8 @@ import java.util.ArrayList;
 import java.util.NoSuchElementException;
 import java.util.Random;
 import java.util.Set;
+import java.util.Timer;
+import java.util.TimerTask;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutorService;
@@ -1051,7 +1053,7 @@ public class TestGenericKeyedObjectPool 
             }
         }
     }
-    
+
     /*
      * Note: This test relies on timing for correct execution. There *should* be
      * enough margin for this to work correctly on most (all?) systems but be
@@ -1062,7 +1064,7 @@ public class TestGenericKeyedObjectPool 
     })
     @Test(timeout=60000)
     public void testBorrowObjectFairness() throws Exception {
-        
+
         int numThreads = 40;
         int maxTotal = 40;
 
@@ -1071,9 +1073,9 @@ public class TestGenericKeyedObjectPool 
         config.setFairness(true);
         config.setLifo(false);
         config.setMaxIdlePerKey(maxTotal);
-        
+
         pool = new GenericKeyedObjectPool<String, String>(factory, config);
-        
+
         // Exhaust the pool
         String[] objects = new String[maxTotal];
         for (int i = 0; i < maxTotal; i++) {
@@ -1093,7 +1095,7 @@ public class TestGenericKeyedObjectPool 
                 fail(e.toString());
             }
         }
-        
+
         // Return objects, other threads should get served in order
         for (int i = 0; i < maxTotal; i++) {
             pool.returnObject("0", objects[i]);
@@ -1567,7 +1569,7 @@ public class TestGenericKeyedObjectPool 
         }
         Assert.assertEquals(nIterations, pool.getDestroyedCount());
     }
-    
+
     // POOL-259
     @Test
     public void testClientWaitStats() throws Exception {
@@ -1584,8 +1586,36 @@ public class TestGenericKeyedObjectPool 
         pool.borrowObject("one");
         // Second borrow does not have to wait on create, average should be about 50
         Assert.assertTrue(pool.getMaxBorrowWaitTimeMillis() >= 100);
-        Assert.assertTrue(pool.getMeanBorrowWaitTimeMillis() < 60);  
-        Assert.assertTrue(pool.getMeanBorrowWaitTimeMillis() > 40);  
+        Assert.assertTrue(pool.getMeanBorrowWaitTimeMillis() < 60);
+        Assert.assertTrue(pool.getMeanBorrowWaitTimeMillis() > 40);
+    }
+
+    // POOL-276
+    @Test
+    public void testValidationOnCreateOnly() throws Exception {
+        factory.enableValidation = true;
+
+        pool.setMaxTotal(1);
+        pool.setTestOnCreate(true);
+        pool.setTestOnBorrow(false);
+        pool.setTestOnReturn(false);
+        pool.setTestWhileIdle(false);
+
+        final String o1 = pool.borrowObject("KEY");
+        Assert.assertEquals("KEY0", o1);
+        Timer t = new Timer();
+        t.schedule(
+                new TimerTask() {
+                    @Override
+                    public void run() {
+                        pool.returnObject("KEY", o1);
+                    }
+                }, 3000);
+
+        String o2 = pool.borrowObject("KEY");
+        Assert.assertEquals("KEY0", o2);
+
+        Assert.assertEquals(1, factory.validateCounter);
     }
 
     /**
@@ -1712,7 +1742,7 @@ public class TestGenericKeyedObjectPool 
         public TestThread(KeyedObjectPool<String,T> pool, int iter) {
             this(pool, iter, 50, 50, true, null, null);
         }
-        
+
         public TestThread(KeyedObjectPool<String,T> pool, int iter, int delay) {
             this(pool, iter, delay, delay, true, null, null);
         }
@@ -1726,7 +1756,7 @@ public class TestGenericKeyedObjectPool 
             _randomDelay = randomDelay;
             _expectedObject = expectedObject;
             _key = key;
-            
+
         }
 
         public boolean complete() {
@@ -1755,7 +1785,7 @@ public class TestGenericKeyedObjectPool 
                     _complete = true;
                     break;
                 }
-                
+
                 if (_expectedObject != null && !_expectedObject.equals(obj)) {
                     _exception = new Exception("Expected: "+_expectedObject+ " found: "+obj);
                     _failed = true;

Modified: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java?rev=1622424&r1=1622423&r2=1622424&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java (original)
+++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java Thu Sep  4 09:00:24 2014
@@ -30,6 +30,8 @@ import java.util.List;
 import java.util.NoSuchElementException;
 import java.util.Random;
 import java.util.Set;
+import java.util.Timer;
+import java.util.TimerTask;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import javax.management.MBeanServer;
@@ -1802,7 +1804,7 @@ public class TestGenericObjectPool exten
                 hurl = exceptionOnActivate;
                 evenTest = evenValid;
                 oddTest = oddValid;
-                counter = validateCounter++;
+                counter = activationCounter++;
             }
             if (hurl) {
                 if (!(counter%2 == 0 ? evenTest : oddTest)) {
@@ -1821,6 +1823,7 @@ public class TestGenericObjectPool exten
             }
         }
         int makeCounter = 0;
+        int activationCounter = 0;
         int validateCounter = 0;
         int activeCount = 0;
         boolean evenValid = true;
@@ -2369,6 +2372,32 @@ public class TestGenericObjectPool exten
         Assert.assertTrue(pool.getMeanBorrowWaitTimeMillis() > 40);
     }
 
+    // POOL-276
+    @Test
+    public void testValidationOnCreateOnly() throws Exception {
+        pool.setMaxTotal(1);
+        pool.setTestOnCreate(true);
+        pool.setTestOnBorrow(false);
+        pool.setTestOnReturn(false);
+        pool.setTestWhileIdle(false);
+
+        final String o1 = pool.borrowObject();
+        Assert.assertEquals("0", o1);
+        Timer t = new Timer();
+        t.schedule(
+                new TimerTask() {
+                    @Override
+                    public void run() {
+                        pool.returnObject(o1);
+                    }
+                }, 3000);
+
+        String o2 = pool.borrowObject();
+        Assert.assertEquals("0", o2);
+
+        Assert.assertEquals(1, factory.validateCounter);
+    }
+
     private static final class DummyFactory
             extends BasePooledObjectFactory<Object> {
         @Override