You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2021/06/22 23:16:52 UTC
[commons-pool] branch master updated: POOL-396: Handle validation
exceptions during eviction. (#85)
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-pool.git
The following commit(s) were added to refs/heads/master by this push:
new 1fe14bb POOL-396: Handle validation exceptions during eviction. (#85)
1fe14bb is described below
commit 1fe14bbb2ea23c98abcfac3019d1daf34568d5ea
Author: Jeremy Kong <je...@hotmail.com>
AuthorDate: Wed Jun 23 00:16:42 2021 +0100
POOL-396: Handle validation exceptions during eviction. (#85)
* POOL-396: Handle validation exceptions during eviction.
* GenericObjectPool fixes
* Add missing brace
* Analogous fix in GenericKeyedObjectPool
* Cleanup test
* Improve tests
* self review
* push?
* CR iteration 2
---
.../commons/pool2/impl/GenericKeyedObjectPool.java | 16 ++++++++++-
.../commons/pool2/impl/GenericObjectPool.java | 16 ++++++++++-
.../pool2/impl/TestGenericKeyedObjectPool.java | 28 ++++++++++++++++++
.../commons/pool2/impl/TestGenericObjectPool.java | 33 ++++++++++++++++++++++
4 files changed, 91 insertions(+), 2 deletions(-)
diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
index a5ae8aa..5ee96e1 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
@@ -1042,9 +1042,23 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
destroyedByEvictorCount.incrementAndGet();
}
if (active) {
- if (!factory.validateObject(evictionKey, underTest)) {
+ boolean validate = false;
+ Throwable validationThrowable = null;
+ try {
+ validate = factory.validateObject(evictionKey, underTest);
+ } catch (final Throwable t) {
+ PoolUtils.checkRethrow(t);
+ validationThrowable = t;
+ }
+ if (!validate) {
destroy(evictionKey, underTest, true, DestroyMode.NORMAL);
destroyedByEvictorCount.incrementAndGet();
+ if (validationThrowable != null) {
+ if (validationThrowable instanceof RuntimeException) {
+ throw (RuntimeException) validationThrowable;
+ }
+ throw (Error) validationThrowable;
+ }
} else {
try {
factory.passivateObject(evictionKey, underTest);
diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
index c97d281..7263577 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
@@ -734,9 +734,23 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
destroyedByEvictorCount.incrementAndGet();
}
if (active) {
- if (!factory.validateObject(underTest)) {
+ boolean validate = false;
+ Throwable validationThrowable = null;
+ try {
+ validate = factory.validateObject(underTest);
+ } catch (final Throwable t) {
+ PoolUtils.checkRethrow(t);
+ validationThrowable = t;
+ }
+ if (!validate) {
destroy(underTest, DestroyMode.NORMAL);
destroyedByEvictorCount.incrementAndGet();
+ if (validationThrowable != null) {
+ if (validationThrowable instanceof RuntimeException) {
+ throw (RuntimeException) validationThrowable;
+ }
+ throw (Error) validationThrowable;
+ }
} else {
try {
factory.passivateObject(underTest);
diff --git a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
index 0366010..5512c0c 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
@@ -167,6 +167,7 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
boolean exceptionOnPassivate = false;
boolean exceptionOnActivate = false;
boolean exceptionOnDestroy = false;
+ boolean exceptionOnValidate = false;
boolean exceptionOnCreate = false;
@@ -246,6 +247,9 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
public void setThrowExceptionOnPassivate(final boolean b) {
exceptionOnPassivate = b;
}
+ public void setThrowExceptionOnValidate(final boolean b) {
+ exceptionOnValidate = b;
+ }
void setValid(final boolean valid) {
evenValid = valid;
oddValid = valid;
@@ -260,6 +264,9 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
@Override
public boolean validateObject(final K key, final PooledObject<String> obj) {
doWait(validateLatency);
+ if (exceptionOnValidate) {
+ throw new RuntimeException("validation failed");
+ }
if (enableValidation) {
return validateCounter++%2 == 0 ? evenValid : oddValid;
}
@@ -1554,6 +1561,27 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
@Test
@Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
+ public void testExceptionInValidationDuringEviction() throws Exception {
+ gkoPool.setMaxIdlePerKey(1);
+ gkoPool.setMinEvictableIdleTime(Duration.ZERO);
+ gkoPool.setTestWhileIdle(true);
+
+ final String obj = gkoPool.borrowObject("one");
+ gkoPool.returnObject("one", obj);
+
+ simpleFactory.setThrowExceptionOnValidate(true);
+ try {
+ gkoPool.evict();
+ fail("Expecting RuntimeException");
+ } catch (RuntimeException e) {
+ // expected
+ }
+ assertEquals(0, gkoPool.getNumActive());
+ assertEquals(0, gkoPool.getNumIdle());
+ }
+
+ @Test
+ @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
public void testFIFO() throws Exception {
gkoPool.setLifo(false);
final String key = "key";
diff --git a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
index b35cbb2..e5e040e 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
@@ -252,6 +252,8 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
boolean exceptionOnDestroy;
+ boolean exceptionOnValidate;
+
boolean enableValidation = true;
long destroyLatency;
@@ -381,6 +383,10 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
exceptionOnPassivate = bool;
}
+ public synchronized void setThrowExceptionOnValidate(final boolean bool) {
+ exceptionOnValidate = bool;
+ }
+
public synchronized void setValid(final boolean valid) {
setEvenValid(valid);
setOddValid(valid);
@@ -397,12 +403,14 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
@Override
public boolean validateObject(final PooledObject<String> obj) {
final boolean validate;
+ final boolean throwException;
final boolean evenTest;
final boolean oddTest;
final long waitLatency;
final int counter;
synchronized(this) {
validate = enableValidation;
+ throwException = exceptionOnValidate;
evenTest = evenValid;
oddTest = oddValid;
counter = validateCounter++;
@@ -411,6 +419,9 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
if (waitLatency > 0) {
doWait(waitLatency);
}
+ if (throwException) {
+ throw new RuntimeException("validation failed");
+ }
if (validate) {
return counter%2 == 0 ? evenTest : oddTest;
}
@@ -1702,6 +1713,28 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
@Test
@Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
+ public void testExceptionInValidationDuringEviction() throws Exception {
+ genericObjectPool.setMaxIdle(1);
+ genericObjectPool.setMinEvictableIdleTime(Duration.ZERO);
+ genericObjectPool.setTestWhileIdle(true);
+
+ String active = genericObjectPool.borrowObject();
+ genericObjectPool.returnObject(active);
+
+ simpleFactory.setThrowExceptionOnValidate(true);
+
+ try {
+ genericObjectPool.evict();
+ fail("Expecting RuntimeException");
+ } catch (RuntimeException e) {
+ // expected
+ }
+ assertEquals(0, genericObjectPool.getNumActive());
+ assertEquals(0, genericObjectPool.getNumIdle());
+ }
+
+ @Test
+ @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
public void testExceptionOnActivateDuringBorrow() throws Exception {
final String obj1 = genericObjectPool.borrowObject();
final String obj2 = genericObjectPool.borrowObject();