You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ps...@apache.org on 2015/05/14 22:05:17 UTC
svn commit: r1679448 - in /commons/proper/pool/trunk/src: changes/
main/java/org/apache/commons/pool2/impl/ test/java/org/apache/commons/pool2/
test/java/org/apache/commons/pool2/impl/
Author: psteitz
Date: Thu May 14 20:05:17 2015
New Revision: 1679448
URL: http://svn.apache.org/r1679448
Log:
Ensured that when an instance that has already been returned to a pool is
returned again, the expected IllegalStateException is generated before the
returning object is re-validated or re-passivated.
JIRI: POOL-285
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/Waiter.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=1679448&r1=1679447&r2=1679448&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/changes/changes.xml (original)
+++ commons/proper/pool/trunk/src/changes/changes.xml Thu May 14 20:05:17 2015
@@ -57,11 +57,16 @@ The <action> type attribute can be add,u
Eliminated the requirement that object equality and hashcodes do not change
while objects are under management by GenericObjectPool or GenericKeyedObjectPool.
</action>
- <action dev="markt" type="fix" issues="POOL-289" due-to="Luke Winkenbach">
+ <action dev="markt" type="fix" issue="POOL-289" due-to="Luke Winkenbach">
Fixed class loading for custom EvictionPolicy implementations that may not
be present in the class loader hierarchy of the Pool classes by falling
back to the class loader of the current class.
</action>
+ <action dev="psteitz" type="fix" issue="POOL-285">
+ Ensured that when an instance that has already been returned to a pool is
+ returned again, the expected IllegalStateException is generated before the
+ returning object is re-validated or re-passivated.
+ </action>
</release>
<release version="2.3" date="2014-12-30" description=
"This is a maintenance release that includes bug fixes and minor enhancements.">
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=1679448&r1=1679447&r2=1679448&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 May 14 20:05:17 2015
@@ -477,6 +477,16 @@ public class GenericKeyedObjectPool<K,T>
throw new IllegalStateException(
"Returned object not currently part of this pool");
}
+
+ synchronized(p) {
+ final PooledObjectState state = p.getState();
+ if (state != PooledObjectState.ALLOCATED) {
+ throw new IllegalStateException(
+ "Object has already been returned to this pool or is invalid");
+ } else {
+ p.markReturning(); // Keep from being marked abandoned (once GKOP does this)
+ }
+ }
long activeTime = p.getActiveTimeMillis();
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=1679448&r1=1679447&r2=1679448&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 May 14 20:05:17 2015
@@ -535,26 +535,23 @@ public class GenericObjectPool<T> extend
@Override
public void returnObject(T obj) {
PooledObject<T> p = allObjects.get(new IdentityWrapper<T>(obj));
-
- if (!isAbandonedConfig()) {
- if (p == null) {
+
+ if (p == null) {
+ if (!isAbandonedConfig()) {
throw new IllegalStateException(
"Returned object not currently part of this pool");
+ } else {
+ return; // Object was abandoned and removed
}
- } else {
- if (p == null) {
- return; // Object was abandoned and removed
+ }
+
+ synchronized(p) {
+ final PooledObjectState state = p.getState();
+ if (state != PooledObjectState.ALLOCATED) {
+ throw new IllegalStateException(
+ "Object has already been returned to this pool or is invalid");
} else {
- // Make sure object is not being reclaimed
- synchronized(p) {
- final PooledObjectState state = p.getState();
- if (state != PooledObjectState.ALLOCATED) {
- throw new IllegalStateException(
- "Object has already been returned to this pool or is invalid");
- } else {
- p.markReturning(); // Keep from being marked abandoned
- }
- }
+ p.markReturning(); // Keep from being marked abandoned
}
}
Modified: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/Waiter.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/Waiter.java?rev=1679448&r1=1679447&r2=1679448&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/Waiter.java (original)
+++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/Waiter.java Thu May 14 20:05:17 2015
@@ -33,6 +33,8 @@ public class Waiter {
private long latency = 0;
private long lastPassivated = 0;
private long lastIdleTimeMs = 0;
+ private long passivationCount = 0;
+ private long validationCount = 0;
private final int id = instanceCount.getAndIncrement();
public Waiter(boolean active, boolean valid, long latency) {
@@ -89,6 +91,7 @@ public class Waiter {
lastIdleTimeMs = currentTime - lastPassivated;
} else { // passivating
lastPassivated = currentTime;
+ passivationCount++;
}
}
@@ -101,6 +104,7 @@ public class Waiter {
}
public boolean isValid() {
+ validationCount++;
return valid;
}
@@ -132,6 +136,20 @@ public class Waiter {
public long getLastIdleTimeMs() {
return lastIdleTimeMs;
}
+
+ /**
+ * @return how many times this instance has been validated
+ */
+ public long getValidationCount() {
+ return validationCount;
+ }
+
+ /**
+ * @return how many times this instance has been passivated
+ */
+ public long getPassivationCount() {
+ return passivationCount;
+ }
@Override
public int hashCode() {
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=1679448&r1=1679447&r2=1679448&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 May 14 20:05:17 2015
@@ -2220,6 +2220,32 @@ public class TestGenericKeyedObjectPool
pool.returnObject("a", s2);
pool.close();
}
+
+ /**
+ * Verifies that returning an object twice (without borrow in between) causes ISE
+ * but does not re-validate or re-passivate the instance.
+ *
+ * JIRA: POOL-285
+ */
+ @Test
+ public void testMultipleReturn() throws Exception {
+ final WaiterFactory<String> factory = new WaiterFactory<String>(0, 0, 0, 0, 0, 0);
+ final GenericKeyedObjectPool<String, Waiter> pool =
+ new GenericKeyedObjectPool<String, Waiter>(factory);
+ pool.setTestOnReturn(true);
+ Waiter waiter = pool.borrowObject("a");
+ pool.returnObject("a",waiter);
+ Assert.assertEquals(1, waiter.getValidationCount());
+ Assert.assertEquals(1, waiter.getPassivationCount());
+ try {
+ pool.returnObject("a",waiter);
+ fail("Expecting IllegalStateException from multiple return");
+ } catch (IllegalStateException ex) {
+ // Exception is expected, now check no repeat validation/passivation
+ Assert.assertEquals(1, waiter.getValidationCount());
+ Assert.assertEquals(1, waiter.getPassivationCount());
+ }
+ }
private static class DummyFactory
extends BaseKeyedPooledObjectFactory<Object,Object> {
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=1679448&r1=1679447&r2=1679448&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 May 14 20:05:17 2015
@@ -47,6 +47,8 @@ import org.apache.commons.pool2.Swallowe
import org.apache.commons.pool2.TestBaseObjectPool;
import org.apache.commons.pool2.VisitTracker;
import org.apache.commons.pool2.VisitTrackerFactory;
+import org.apache.commons.pool2.Waiter;
+import org.apache.commons.pool2.WaiterFactory;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
@@ -2437,6 +2439,31 @@ public class TestGenericObjectPool exten
pool.returnObject(s2);
pool.close();
}
+
+ /**
+ * Verifies that returning an object twice (without borrow in between) causes ISE
+ * but does not re-validate or re-passivate the instance.
+ *
+ * JIRA: POOL-285
+ */
+ @Test
+ public void testMultipleReturn() throws Exception {
+ final WaiterFactory<String> factory = new WaiterFactory<String>(0, 0, 0, 0, 0, 0);
+ final GenericObjectPool<Waiter> pool = new GenericObjectPool<Waiter>(factory);
+ pool.setTestOnReturn(true);
+ Waiter waiter = pool.borrowObject();
+ pool.returnObject(waiter);
+ Assert.assertEquals(1, waiter.getValidationCount());
+ Assert.assertEquals(1, waiter.getPassivationCount());
+ try {
+ pool.returnObject(waiter);
+ fail("Expecting IllegalStateException from multiple return");
+ } catch (IllegalStateException ex) {
+ // Exception is expected, now check no repeat validation/passivation
+ Assert.assertEquals(1, waiter.getValidationCount());
+ Assert.assertEquals(1, waiter.getPassivationCount());
+ }
+ }
private static final class DummyFactory
extends BasePooledObjectFactory<Object> {