Skip to content
Permalink
Browse files
Getting a PooledObject's active duration returns a negative duration
when the object is borrowed but not returned. Affects:
- PooledObject.getActiveDuration()
- PooledObject.getActiveTime()
- PooledObject.getActiveTimeMillis()
  • Loading branch information
Gary Gregory committed Aug 12, 2021
1 parent 43d7652 commit 3f8c121f789db232a2bb8c67488f3c0251c20d9b
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 7 deletions.
@@ -43,8 +43,14 @@ The <action> type attribute can be add,update,fix,remove.
<title>Apache Commons Pool Release Notes</title>
</properties>
<body>
<release version="2.11.1" date="2021-08-08" description="This is a maintenance release (Java 8).">
<release version="2.11.1" date="2021-08-DD" description="This is a maintenance release (Java 8).">
<!-- FIXES -->
<action dev="ggregory" type="fix" due-to="Gary Gregory">
Getting a PooledObject's active duration returns a negative duration when the object is borrowed but not returned. Affects:
- PooledObject.getActiveDuration()
- PooledObject.getActiveTime()
- PooledObject.getActiveTimeMillis()
</action>
<action dev="ggregory" type="fix" due-to="Gary Gregory">
Fix field label in BaseGenericObjectPool toString() builder: From timeBetweenEvictionRunsMillis to durationBetweenEvictionRuns.
</action>
@@ -89,7 +89,7 @@ default Duration getActiveDuration() {
// @formatter:off
return lastReturnInstant.isAfter(lastBorrowInstant) ?
Duration.between(lastBorrowInstant, lastReturnInstant) :
Duration.between(Instant.now(), lastBorrowInstant);
Duration.between(lastBorrowInstant, Instant.now());
// @formatter:on
}

@@ -369,6 +369,10 @@ public T borrowObject(final Duration borrowMaxWaitDuration) throws Exception {
return p.getObject();
}

PooledObject<T> getPooledObject(final T obj) {
return allObjects.get(new IdentityWrapper<>(obj));
}

@Override
String getStatsString() {
// Simply listed in AB order.
@@ -924,7 +928,7 @@ public void invalidateObject(final T obj) throws Exception {
*/
@Override
public void invalidateObject(final T obj, final DestroyMode destroyMode) throws Exception {
final PooledObject<T> p = allObjects.get(new IdentityWrapper<>(obj));
final PooledObject<T> p = getPooledObject(obj);
if (p == null) {
if (isAbandonedConfig()) {
return;
@@ -1012,7 +1016,7 @@ private void removeAbandoned(final AbandonedConfig abandonedConfig) {
*/
@Override
public void returnObject(final T obj) {
final PooledObject<T> p = allObjects.get(new IdentityWrapper<>(obj));
final PooledObject<T> p = getPooledObject(obj);

if (p == null) {
if (!isAbandonedConfig()) {
@@ -1173,7 +1177,7 @@ protected void toStringAppendFields(final StringBuilder builder) {
public void use(final T pooledObject) {
final AbandonedConfig abandonedCfg = this.abandonedConfig;
if (abandonedCfg != null && abandonedCfg.getUseUsageTracking()) {
allObjects.get(new IdentityWrapper<>(pooledObject)).use();
getPooledObject(pooledObject).use();
}
}

@@ -17,6 +17,7 @@
package org.apache.commons.pool2.impl;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.ArrayList;
import java.util.List;
@@ -27,9 +28,22 @@

import org.junit.jupiter.api.Test;


/**
* Tests {@link DefaultPooledObject}.
*/
public class TestDefaultPooledObject {

@Test
public void testGetActiveDuration() throws InterruptedException {
final DefaultPooledObject<Object> dpo = new DefaultPooledObject<>(new Object());
// Sleep MUST be "long enough" to test that we are not returning a negative time.
Thread.sleep(200);
assertFalse(dpo.getActiveDuration().isNegative());
assertFalse(dpo.getActiveDuration().isZero());
assertEquals(dpo.getActiveDuration().toMillis(), dpo.getActiveTimeMillis());
assertEquals(dpo.getActiveDuration(), dpo.getActiveTime());
}

/**
* JIRA: POOL-279
* @throws Exception May occur in some failure modes
@@ -76,5 +90,4 @@ public void testGetIdleTimeMillis() throws Exception {
assertFalse(negativeIdleTimeReturned.get(),
"DefaultPooledObject.getIdleTimeMillis() returned a negative value");
}

}
@@ -1897,6 +1897,28 @@ public void testFIFO() throws Exception {
assertEquals( "4", genericObjectPool.borrowObject(),"new-4");
}

@Test
public void testGetActiveDuration() throws Exception {
// Borrow
final String object = genericObjectPool.borrowObject();
final PooledObject<String> dpo = genericObjectPool.getPooledObject(object);

// Sleep MUST be "long enough" to detect that more than 0 milliseconds have elapsed.
Thread.sleep(200);
assertFalse(dpo.getActiveDuration().isNegative());
assertFalse(dpo.getActiveDuration().isZero());
assertEquals(dpo.getActiveDuration().toMillis(), dpo.getActiveTimeMillis());
assertEquals(dpo.getActiveDuration(), dpo.getActiveTime());

// Return
genericObjectPool.returnObject(object);

assertFalse(dpo.getActiveDuration().isNegative());
assertFalse(dpo.getActiveDuration().isZero());
assertEquals(dpo.getActiveDuration().toMillis(), dpo.getActiveTimeMillis());
assertEquals(dpo.getActiveDuration(), dpo.getActiveTime());
}

@Test
public void testGetFactoryType_DefaultPooledObjectFactory() {
try (final GenericObjectPool<String> pool = new GenericObjectPool<>(createDefaultPooledObjectFactory())) {

0 comments on commit 3f8c121

Please sign in to comment.