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/08/12 21:03:39 UTC

[commons-pool] branch master updated: 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()

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 3f8c121  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()
3f8c121 is described below

commit 3f8c121f789db232a2bb8c67488f3c0251c20d9b
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Thu Aug 12 17:03:36 2021 -0400

    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()
---
 src/changes/changes.xml                            |  8 +++++++-
 .../org/apache/commons/pool2/PooledObject.java     |  2 +-
 .../commons/pool2/impl/GenericObjectPool.java      | 10 +++++++---
 .../pool2/impl/TestDefaultPooledObject.java        | 17 +++++++++++++++--
 .../commons/pool2/impl/TestGenericObjectPool.java  | 22 ++++++++++++++++++++++
 5 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 2cb5650..035fa7b 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -43,9 +43,15 @@ 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>
     <action dev="ggregory" type="fix" due-to="Gary Gregory">
diff --git a/src/main/java/org/apache/commons/pool2/PooledObject.java b/src/main/java/org/apache/commons/pool2/PooledObject.java
index 608f990..83ac35c 100644
--- a/src/main/java/org/apache/commons/pool2/PooledObject.java
+++ b/src/main/java/org/apache/commons/pool2/PooledObject.java
@@ -89,7 +89,7 @@ public interface PooledObject<T> extends Comparable<PooledObject<T>> {
         // @formatter:off
         return lastReturnInstant.isAfter(lastBorrowInstant) ?
                 Duration.between(lastBorrowInstant, lastReturnInstant) :
-                Duration.between(Instant.now(), lastBorrowInstant);
+                Duration.between(lastBorrowInstant, Instant.now());
         // @formatter:on
     }
 
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 e7bf0a2..d732fe1 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
@@ -369,6 +369,10 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
         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 class GenericObjectPool<T> extends BaseGenericObjectPool<T>
      */
     @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 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
      */
     @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 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
     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();
         }
     }
 
diff --git a/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObject.java b/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObject.java
index bf0cf1d..9dd7c97 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObject.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObject.java
@@ -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 java.util.concurrent.atomic.AtomicBoolean;
 
 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 class TestDefaultPooledObject {
         assertFalse(negativeIdleTimeReturned.get(),
                 "DefaultPooledObject.getIdleTimeMillis() returned a negative value");
     }
-
 }
\ No newline at end of file
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 dcad0ac..96ff12f 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
@@ -1898,6 +1898,28 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
     }
 
     @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())) {
             assertNotNull((pool.getFactoryType()));