diff --git a/src/main/java/org/apache/commons/pool3/PooledObject.java b/src/main/java/org/apache/commons/pool3/PooledObject.java index 34de71a5b..16923602c 100644 --- a/src/main/java/org/apache/commons/pool3/PooledObject.java +++ b/src/main/java/org/apache/commons/pool3/PooledObject.java @@ -255,4 +255,19 @@ default void setRequireFullStackTrace(final boolean requireFullStackTrace) { */ void use(); + /** + * Acquires a lock on this PooledObject. + */ + @SuppressWarnings("no-ops") + default void lock() { + + } + + /** + * Release a lock on this PooledObject. + */ + @SuppressWarnings("no-ops") + default void unlock() { + + } } diff --git a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java index 0095a20f0..36b00bc8c 100644 --- a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java @@ -34,6 +34,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import javax.management.InstanceAlreadyExistsException; @@ -379,7 +380,7 @@ public String toString() { final Object closeLock = new Object(); volatile boolean closed; - final Object evictionLock = new Object(); + final ReentrantLock evictionLock = new ReentrantLock (); private Evictor evictor; // @GuardedBy("evictionLock") EvictionIterator evictionIterator; // @GuardedBy("evictionLock") @@ -496,12 +497,15 @@ ArrayList> createRemoveList(final AbandonedConfig abandonedConfi final Instant timeout = Instant.now().minus(abandonedConfig.getRemoveAbandonedTimeoutDuration()); final ArrayList> remove = new ArrayList<>(); allObjects.values().forEach(pooledObject -> { - synchronized (pooledObject) { + pooledObject.lock(); + try { if (pooledObject.getState() == PooledObjectState.ALLOCATED && pooledObject.getLastUsedInstant().compareTo(timeout) <= 0) { pooledObject.markAbandoned(); remove.add(pooledObject); } + } finally { + pooledObject.unlock(); } }); return remove; @@ -1215,11 +1219,14 @@ final void jmxUnregister() { * @param pooledObject instance to return to the keyed pool */ protected void markReturningState(final PooledObject pooledObject) { - synchronized (pooledObject) { + pooledObject.lock(); + try { if (pooledObject.getState() != PooledObjectState.ALLOCATED) { throw new IllegalStateException("Object has already been returned to this pool or is invalid"); } pooledObject.markReturning(); // Keep from being marked abandoned + } finally { + pooledObject.unlock(); } } @@ -1617,7 +1624,8 @@ public final void setTestWhileIdle(final boolean testWhileIdle) { * @param delay duration before start and between eviction runs. */ final void startEvictor(final Duration delay) { - synchronized (evictionLock) { + evictionLock.lock(); + try { final boolean isPositiverDelay = PoolImplUtils.isPositive(delay); if (evictor == null) { // Starting evictor for the first time or after a cancel if (isPositiverDelay) { // Starting new evictor @@ -1635,6 +1643,8 @@ final void startEvictor(final Duration delay) { } else { // Stopping evictor EvictionTimer.cancel(evictor, evictorShutdownTimeoutDuration, false); } + } finally { + evictionLock.unlock(); } } diff --git a/src/main/java/org/apache/commons/pool3/impl/DefaultPooledObject.java b/src/main/java/org/apache/commons/pool3/impl/DefaultPooledObject.java index de8268057..fe0ba3bfd 100644 --- a/src/main/java/org/apache/commons/pool3/impl/DefaultPooledObject.java +++ b/src/main/java/org/apache/commons/pool3/impl/DefaultPooledObject.java @@ -22,6 +22,7 @@ import java.time.Instant; import java.util.Deque; import java.util.Objects; +import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.pool3.PooledObject; import org.apache.commons.pool3.PooledObjectState; @@ -44,6 +45,7 @@ public class DefaultPooledObject implements PooledObject { private PooledObjectState state = PooledObjectState.IDLE; // @GuardedBy("this") to ensure transitions are valid private final Clock systemClock = Clock.systemUTC(); private final Instant createInstant = now(); + private final ReentrantLock lock = new ReentrantLock(); private volatile Instant lastBorrowInstant = createInstant; private volatile Instant lastUseInstant = createInstant; @@ -69,24 +71,29 @@ public DefaultPooledObject(final T object) { * @return {@code true} if the original state was {@link PooledObjectState#IDLE IDLE} */ @Override - public synchronized boolean allocate() { - if (state == PooledObjectState.IDLE) { - state = PooledObjectState.ALLOCATED; - lastBorrowInstant = now(); - lastUseInstant = lastBorrowInstant; - borrowedCount++; - if (logAbandoned) { - borrowedBy.fillInStackTrace(); + public boolean allocate() { + lock(); + try { + if (state == PooledObjectState.IDLE) { + state = PooledObjectState.ALLOCATED; + lastBorrowInstant = now(); + lastUseInstant = lastBorrowInstant; + borrowedCount++; + if (logAbandoned) { + borrowedBy.fillInStackTrace(); + } + return true; } - return true; - } - if (state == PooledObjectState.EVICTION) { - // TODO Allocate anyway and ignore eviction test - state = PooledObjectState.EVICTION_RETURN_TO_HEAD; + if (state == PooledObjectState.EVICTION) { + // TODO Allocate anyway and ignore eviction test + state = PooledObjectState.EVICTION_RETURN_TO_HEAD; + } + // TODO if validating and testOnBorrow == true then pre-allocate for + // performance + return false; + } finally { + unlock(); } - // TODO if validating and testOnBorrow == true then pre-allocate for - // performance - return false; } @Override @@ -111,30 +118,38 @@ public int compareTo(final PooledObject other) { * or {@link PooledObjectState#RETURNING RETURNING}. */ @Override - public synchronized boolean deallocate() { - if (state == PooledObjectState.ALLOCATED || state == PooledObjectState.RETURNING) { - state = PooledObjectState.IDLE; - lastReturnInstant = now(); - borrowedBy.clear(); - return true; + public boolean deallocate() { + lock(); + try { + if (state == PooledObjectState.ALLOCATED || state == PooledObjectState.RETURNING) { + state = PooledObjectState.IDLE; + lastReturnInstant = now(); + borrowedBy.clear(); + return true; + } + return false; + } finally { + unlock(); } - - return false; } @Override - public synchronized boolean endEvictionTest( + public boolean endEvictionTest( final Deque> idleQueue) { - if (state == PooledObjectState.EVICTION) { - state = PooledObjectState.IDLE; - return true; - } - if (state == PooledObjectState.EVICTION_RETURN_TO_HEAD) { - state = PooledObjectState.IDLE; - idleQueue.offerFirst(this); + lock(); + try { + if (state == PooledObjectState.EVICTION) { + state = PooledObjectState.IDLE; + return true; + } + if (state == PooledObjectState.EVICTION_RETURN_TO_HEAD) { + state = PooledObjectState.IDLE; + idleQueue.offerFirst(this); + } + return false; + } finally { + unlock(); } - - return false; } /** @@ -198,32 +213,52 @@ public T getObject() { * @return state */ @Override - public synchronized PooledObjectState getState() { - return state; + public PooledObjectState getState() { + lock(); + try { + return state; + } finally { + unlock(); + } } /** * Sets the state to {@link PooledObjectState#INVALID INVALID}. */ @Override - public synchronized void invalidate() { - state = PooledObjectState.INVALID; + public void invalidate() { + lock(); + try { + state = PooledObjectState.INVALID; + } finally { + unlock(); + } } /** * Marks the pooled object as {@link PooledObjectState#ABANDONED ABANDONED}. */ @Override - public synchronized void markAbandoned() { - state = PooledObjectState.ABANDONED; + public void markAbandoned() { + lock(); + try { + state = PooledObjectState.ABANDONED; + } finally { + unlock(); + } } /** * Marks the pooled object as {@link PooledObjectState#RETURNING RETURNING}. */ @Override - public synchronized void markReturning() { - state = PooledObjectState.RETURNING; + public void markReturning() { + lock(); + try { + state = PooledObjectState.RETURNING; + } finally { + unlock(); + } } /** @@ -269,12 +304,17 @@ public void setRequireFullStackTrace(final boolean requireFullStackTrace) { } @Override - public synchronized boolean startEvictionTest() { - if (state == PooledObjectState.IDLE) { - state = PooledObjectState.EVICTION; - return true; + public boolean startEvictionTest() { + lock(); + try { + if (state == PooledObjectState.IDLE) { + state = PooledObjectState.EVICTION; + return true; + } + return false; + } finally { + unlock(); } - return false; } @Override @@ -283,8 +323,11 @@ public String toString() { result.append("Object: "); result.append(Objects.toString(object)); result.append(", State: "); - synchronized (this) { + lock(); + try { result.append(state.toString()); + } finally { + unlock(); } return result.toString(); // TODO add other attributes @@ -296,4 +339,12 @@ public void use() { usedBy.fillInStackTrace(); } + public void lock() { + lock.lock(); + } + + public void unlock() { + lock.unlock(); + } + } \ No newline at end of file diff --git a/src/main/java/org/apache/commons/pool3/impl/GenericKeyedObjectPool.java b/src/main/java/org/apache/commons/pool3/impl/GenericKeyedObjectPool.java index 96ef8e8ff..55f689a38 100644 --- a/src/main/java/org/apache/commons/pool3/impl/GenericKeyedObjectPool.java +++ b/src/main/java/org/apache/commons/pool3/impl/GenericKeyedObjectPool.java @@ -881,7 +881,8 @@ private boolean destroy(final K key, final PooledObject toDestroy, final bool try { boolean isIdle; - synchronized (toDestroy) { + toDestroy.lock(); + try { // Check idle state directly isIdle = toDestroy.getState().equals(PooledObjectState.IDLE); // If idle, not under eviction test, or always is true, remove instance, @@ -889,6 +890,8 @@ private boolean destroy(final K key, final PooledObject toDestroy, final bool if (isIdle || always) { isIdle = objectDeque.getIdleObjects().remove(toDestroy); } + } finally { + toDestroy.unlock(); } if (isIdle || always) { objectDeque.getAllObjects().remove(IdentityWrapper.on(toDestroy)); @@ -979,7 +982,8 @@ public void evict() throws E { PooledObject underTest = null; final EvictionPolicy evictionPolicy = getEvictionPolicy(); - synchronized (evictionLock) { + evictionLock.lock(); + try { final EvictionConfig evictionConfig = new EvictionConfig( getMinEvictableIdleDuration(), getSoftMinEvictableIdleDuration(), @@ -1101,6 +1105,8 @@ public void evict() throws E { // states are used } } + } finally { + evictionLock.unlock(); } } final AbandonedConfig ac = this.abandonedConfig; @@ -1333,11 +1339,14 @@ public void invalidateObject(final K key, final T obj, final DestroyMode destroy if (p == null) { throw new IllegalStateException(appendStats("Object not currently part of this pool")); } - synchronized (p) { + p.lock(); + try { if (p.getState() != PooledObjectState.INVALID) { destroy(key, p, true, destroyMode); reuseCapacity(); } + } finally { + p.unlock(); } } diff --git a/src/main/java/org/apache/commons/pool3/impl/GenericObjectPool.java b/src/main/java/org/apache/commons/pool3/impl/GenericObjectPool.java index 430b8d0fb..4a61cc64e 100644 --- a/src/main/java/org/apache/commons/pool3/impl/GenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool3/impl/GenericObjectPool.java @@ -689,7 +689,8 @@ public void evict() throws E { PooledObject underTest = null; final EvictionPolicy evictionPolicy = getEvictionPolicy(); - synchronized (evictionLock) { + evictionLock.lock(); + try { final EvictionConfig evictionConfig = new EvictionConfig( getMinEvictableIdleDuration(), getSoftMinEvictableIdleDuration(), @@ -785,6 +786,8 @@ public void evict() throws E { // states are used } } + } finally { + evictionLock.unlock(); } } final AbandonedConfig ac = this.abandonedConfig; @@ -955,10 +958,13 @@ public void invalidateObject(final T obj, final DestroyMode destroyMode) throws } throw new IllegalStateException("Invalidated object not currently part of this pool"); } - synchronized (p) { + p.lock(); + try { if (p.getState() != PooledObjectState.INVALID) { destroy(p, destroyMode); } + } finally { + p.unlock(); } ensureIdle(1, false); }