From dce040eef545b781c000e583568e761b19f4800c Mon Sep 17 00:00:00 2001 From: jieyebing Date: Tue, 12 Jun 2018 08:11:52 +0800 Subject: [PATCH 1/2] Refactor PooledObject and BaseGenericObjectPool --- .../apache/commons/pool2/PooledObject.java | 27 +++--------- .../pool2/impl/BaseGenericObjectPool.java | 42 +++++++++++++++++++ .../pool2/impl/DefaultPooledObject.java | 36 ++++------------ .../pool2/impl/GenericKeyedObjectPool.java | 4 +- .../commons/pool2/impl/GenericObjectPool.java | 4 +- 5 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/main/java/org/apache/commons/pool2/PooledObject.java b/src/main/java/org/apache/commons/pool2/PooledObject.java index 9fa2872c5..22c2e1f65 100644 --- a/src/main/java/org/apache/commons/pool2/PooledObject.java +++ b/src/main/java/org/apache/commons/pool2/PooledObject.java @@ -17,7 +17,6 @@ package org.apache.commons.pool2; import java.io.PrintWriter; -import java.util.Deque; /** * Defines the wrapper that is used to track the additional information, such as @@ -117,26 +116,6 @@ public interface PooledObject extends Comparable> { @Override String toString(); - /** - * Attempts to place the pooled object in the - * {@link PooledObjectState#EVICTION} state. - * - * @return true if the object was placed in the - * {@link PooledObjectState#EVICTION} state otherwise - * false - */ - boolean startEvictionTest(); - - /** - * Called to inform the object that the eviction test has ended. - * - * @param idleQueue The queue of idle objects to which the object should be - * returned - * - * @return Currently not used - */ - boolean endEvictionTest(Deque> idleQueue); - /** * Allocates the object. * @@ -199,6 +178,12 @@ public interface PooledObject extends Comparable> { */ PooledObjectState getState(); + /** + * Sets the state of this object. + * @return state + */ + void setState(PooledObjectState newState); + /** * Marks the pooled object as abandoned. */ diff --git a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java index aaaa7bc3e..5a12d6366 100644 --- a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java @@ -761,6 +761,48 @@ final void startEvictor(final long delay) { } } + /** + * Attempts to place the pooled object in the + * {@link PooledObjectState#EVICTION} state. + * + * @param pooledObject instance to start to evict test. + * @return true if the object was placed in the + * {@link PooledObjectState#EVICTION} state otherwise + * false + */ + protected boolean startEvictionTest(PooledObject pooledObject) { + synchronized (pooledObject) { + if (pooledObject.getState() == PooledObjectState.IDLE) { + pooledObject.setState(PooledObjectState.EVICTION); + return true; + } + return false; + } + } + + /** + * Called to inform the object that the eviction test has ended. + * @param idleObjects The queue of idle objects to which the object should be + * returned + * @param pooledObject instance to return to the queue. + * @return Currently not used + */ + protected boolean endEvictionTest(PooledObject pooledObject, Deque> idleObjects) { + synchronized (pooledObject) { + if(pooledObject.getState() == PooledObjectState.EVICTION) { + pooledObject.setState(PooledObjectState.IDLE); + return true; + } else if(pooledObject.getState() == PooledObjectState.EVICTION_RETURN_TO_HEAD) { + pooledObject.setState(PooledObjectState.IDLE); + if (!idleObjects.offerFirst(pooledObject)) { + // TODO - Should never happen + } + } + } + return false; + } + + /** * Tries to ensure that the configured minimum number of idle instances are * available in the pool. diff --git a/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java b/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java index a3ea90240..be7ca848c 100644 --- a/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java +++ b/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java @@ -21,7 +21,6 @@ import org.apache.commons.pool2.TrackedUse; import java.io.PrintWriter; -import java.util.Deque; /** * This wrapper is used to track the additional information, such as state, for @@ -150,32 +149,6 @@ public String toString() { // TODO add other attributes } - @Override - public synchronized boolean startEvictionTest() { - if (state == PooledObjectState.IDLE) { - state = PooledObjectState.EVICTION; - return true; - } - - return false; - } - - @Override - public synchronized boolean endEvictionTest( - final Deque> idleQueue) { - if (state == PooledObjectState.EVICTION) { - state = PooledObjectState.IDLE; - return true; - } else if (state == PooledObjectState.EVICTION_RETURN_TO_HEAD) { - state = PooledObjectState.IDLE; - if (!idleQueue.offerFirst(this)) { - // TODO - Should never happen - } - } - - return false; - } - /** * Allocates the object. * @@ -253,6 +226,15 @@ public synchronized PooledObjectState getState() { return state; } + /** + * {@inheritDoc} + * @param newState + */ + @Override + public synchronized void setState(PooledObjectState newState) { + state = newState; + } + /** * Marks the pooled object as abandoned. */ 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 e80cd7e4d..d96ac7116 100644 --- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java +++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java @@ -924,7 +924,7 @@ public void evict() throws Exception { continue; } - if (!underTest.startEvictionTest()) { + if (!startEvictionTest(underTest)) { // Object was borrowed in another thread // Don't count this as an eviction test so reduce i; i--; @@ -974,7 +974,7 @@ public void evict() throws Exception { } } } - if (!underTest.endEvictionTest(idleObjects)) { + if (!endEvictionTest(underTest, idleObjects)) { // TODO - May need to add code here once additional // states are used } 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 c41b5c278..03440fae6 100644 --- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java @@ -748,7 +748,7 @@ public void evict() throws Exception { continue; } - if (!underTest.startEvictionTest()) { + if (!startEvictionTest(underTest)) { // Object was borrowed in another thread // Don't count this as an eviction test so reduce i; i--; @@ -798,7 +798,7 @@ public void evict() throws Exception { } } } - if (!underTest.endEvictionTest(idleObjects)) { + if (!endEvictionTest(underTest, idleObjects)) { // TODO - May need to add code here once additional // states are used } From cc0a424730a424759aae55e77df8b9b70d7ffa93 Mon Sep 17 00:00:00 2001 From: jieyebing Date: Tue, 12 Jun 2018 08:42:34 +0800 Subject: [PATCH 2/2] Keep PooledObject's startEvictionTest and endEvictionTest methods, but mark Deprecated --- .../apache/commons/pool2/PooledObject.java | 23 ++++++++++++++++ .../pool2/impl/DefaultPooledObject.java | 27 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/src/main/java/org/apache/commons/pool2/PooledObject.java b/src/main/java/org/apache/commons/pool2/PooledObject.java index 22c2e1f65..cc4f49f70 100644 --- a/src/main/java/org/apache/commons/pool2/PooledObject.java +++ b/src/main/java/org/apache/commons/pool2/PooledObject.java @@ -17,6 +17,7 @@ package org.apache.commons.pool2; import java.io.PrintWriter; +import java.util.Deque; /** * Defines the wrapper that is used to track the additional information, such as @@ -116,6 +117,28 @@ public interface PooledObject extends Comparable> { @Override String toString(); + /** + * Attempts to place the pooled object in the + * {@link PooledObjectState#EVICTION} state. + * + * @return true if the object was placed in the + * {@link PooledObjectState#EVICTION} state otherwise + * false + */ + @Deprecated + boolean startEvictionTest(); + + /** + * Called to inform the object that the eviction test has ended. + * + * @param idleQueue The queue of idle objects to which the object should be + * returned + * + * @return Currently not used + */ + @Deprecated + boolean endEvictionTest(Deque> idleQueue); + /** * Allocates the object. * diff --git a/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java b/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java index be7ca848c..c5f1b0afc 100644 --- a/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java +++ b/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java @@ -21,6 +21,7 @@ import org.apache.commons.pool2.TrackedUse; import java.io.PrintWriter; +import java.util.Deque; /** * This wrapper is used to track the additional information, such as state, for @@ -149,6 +150,32 @@ public String toString() { // TODO add other attributes } + @Override + public synchronized boolean startEvictionTest() { + if (state == PooledObjectState.IDLE) { + state = PooledObjectState.EVICTION; + return true; + } + + return false; + } + + @Override + public synchronized boolean endEvictionTest( + final Deque> idleQueue) { + if (state == PooledObjectState.EVICTION) { + state = PooledObjectState.IDLE; + return true; + } else if (state == PooledObjectState.EVICTION_RETURN_TO_HEAD) { + state = PooledObjectState.IDLE; + if (!idleQueue.offerFirst(this)) { + // TODO - Should never happen + } + } + + return false; + } + /** * Allocates the object. *