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 2019/07/22 21:27:29 UTC
[commons-pool] branch master updated: Move validation for newly
created objects into create(). Fixes POOL-361. (#23)
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 d9cf897 Move validation for newly created objects into create(). Fixes POOL-361. (#23)
d9cf897 is described below
commit d9cf8976af4e3a246f197dad71c5eb7caeca66b5
Author: Phil Steitz <ps...@apache.org>
AuthorDate: Mon Jul 22 14:27:25 2019 -0700
Move validation for newly created objects into create(). Fixes POOL-361. (#23)
---
.../commons/pool2/impl/GenericKeyedObjectPool.java | 7 ++++-
.../commons/pool2/impl/GenericObjectPool.java | 6 ++++-
.../pool2/impl/TestGenericKeyedObjectPool.java | 30 ++++++++++++++++++++++
.../commons/pool2/impl/TestGenericObjectPool.java | 28 ++++++++++++++++++++
4 files changed, 69 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 0622bfb..562e1ac 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
@@ -383,7 +383,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
throw nsee;
}
}
- if (p != null && (getTestOnBorrow() || create && getTestOnCreate())) {
+ if (p != null && getTestOnBorrow()) {
boolean validate = false;
Throwable validationThrowable = null;
try {
@@ -1039,6 +1039,11 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
PooledObject<T> p = null;
try {
p = factory.makeObject(key);
+ if (getTestOnCreate() && !factory.validateObject(key, p)) {
+ numTotal.decrementAndGet();
+ objectDeque.getCreateCount().decrementAndGet();
+ return null;
+ }
} catch (final Exception e) {
numTotal.decrementAndGet();
objectDeque.getCreateCount().decrementAndGet();
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 695f611..ffe22b1 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
@@ -465,7 +465,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
throw nsee;
}
}
- if (p != null && (getTestOnBorrow() || create && getTestOnCreate())) {
+ if (p != null && getTestOnBorrow()) {
boolean validate = false;
Throwable validationThrowable = null;
try {
@@ -887,6 +887,10 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
final PooledObject<T> p;
try {
p = factory.makeObject();
+ if (getTestOnCreate() && !factory.validateObject(p)) {
+ createCount.decrementAndGet();
+ return null;
+ }
} catch (final Throwable e) {
createCount.decrementAndGet();
throw e;
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 ae18c90..7b5b5ac 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
@@ -2275,6 +2275,36 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
Assert.assertFalse(testB._failed);
}
+ // Pool-361
+ @Test
+ public void testValidateOnCreate() throws Exception {
+ gkoPool.setTestOnCreate(true);
+ simpleFactory.setValidationEnabled(true);
+ gkoPool.addObject("one");
+ Assert.assertEquals(1, simpleFactory.validateCounter);
+ }
+
+ @Test
+ public void testValidateOnCreateFailure() throws Exception {
+ gkoPool.setTestOnCreate(true);
+ gkoPool.setTestOnBorrow(false);
+ gkoPool.setMaxTotal(2);
+ simpleFactory.setValidationEnabled(true);
+ simpleFactory.setValid(false);
+ // Make sure failed validations do not leak capacity
+ gkoPool.addObject("one");
+ gkoPool.addObject("one");
+ assertEquals(0, gkoPool.getNumIdle());
+ assertEquals(0, gkoPool.getNumActive());
+ simpleFactory.setValid(true);
+ final String obj = gkoPool.borrowObject("one");
+ assertNotNull(obj);
+ gkoPool.addObject("one");
+ // Should have one idle, one out now
+ assertEquals(1, gkoPool.getNumIdle());
+ assertEquals(1, gkoPool.getNumActive());
+ }
+
private static class DummyFactory
extends BaseKeyedPooledObjectFactory<Object,Object> {
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 d9be87f..27c81b2 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
@@ -2839,6 +2839,34 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
Assert.assertEquals(1, simpleFactory.validateCounter);
}
+ // Pool-361
+ @Test
+ public void testValidateOnCreate() throws Exception {
+ genericObjectPool.setTestOnCreate(true);
+ genericObjectPool.addObject();
+ Assert.assertEquals(1, simpleFactory.validateCounter);
+ }
+
+ @Test
+ public void testValidateOnCreateFailure() throws Exception {
+ genericObjectPool.setTestOnCreate(true);
+ genericObjectPool.setTestOnBorrow(false);
+ genericObjectPool.setMaxTotal(2);
+ simpleFactory.setValid(false);
+ // Make sure failed validations do not leak capacity
+ genericObjectPool.addObject();
+ genericObjectPool.addObject();
+ assertEquals(0, genericObjectPool.getNumIdle());
+ assertEquals(0, genericObjectPool.getNumActive());
+ simpleFactory.setValid(true);
+ final String obj = genericObjectPool.borrowObject();
+ assertNotNull(obj);
+ genericObjectPool.addObject();
+ // Should have one idle, one out now
+ assertEquals(1, genericObjectPool.getNumIdle());
+ assertEquals(1, genericObjectPool.getNumActive());
+ }
+
@Test(timeout = 60000)
public void testWhenExhaustedBlock() throws Exception {
genericObjectPool.setMaxTotal(1);