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
