From 75a75cd6d7571143fd065d9a41395d7590dbc701 Mon Sep 17 00:00:00 2001
From: jieyebing <jieyebing@gmail.com>
Date: Mon, 11 Jun 2018 01:07:52 +0800
Subject: [PATCH] code review

---
 .gitignore                                    |  2 +
 .../pool2/impl/BaseGenericObjectPool.java     | 16 ++++++++
 .../pool2/impl/GenericKeyedObjectPool.java    | 39 ++++++++-----------
 .../commons/pool2/impl/GenericObjectPool.java |  9 +----
 4 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/.gitignore b/.gitignore
index 35c0dd076..443c59a66 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,5 @@ site-content
 /.project
 /.settings/
 /bin/
+.idea/
+*.iml
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 372cf877a..aaaa7bc3e 100644
--- a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
@@ -40,6 +40,7 @@
 
 import org.apache.commons.pool2.BaseObject;
 import org.apache.commons.pool2.PooledObject;
+import org.apache.commons.pool2.PooledObjectState;
 import org.apache.commons.pool2.SwallowedExceptionListener;
 
 /**
@@ -966,6 +967,21 @@ final void updateStatsReturn(final long activeTime) {
         activeTimes.add(activeTime);
     }
 
+    /**
+     * Marks the object as returning to the pool.
+     * @param pooledObject instance to return to the keyed pool
+     */
+    protected void markReturningState(PooledObject<T> pooledObject) {
+        synchronized(pooledObject) {
+            final PooledObjectState state = pooledObject.getState();
+            if (state != PooledObjectState.ALLOCATED) {
+                throw new IllegalStateException(
+                        "Object has already been returned to this pool or is invalid");
+            }
+            pooledObject.markReturning(); // Keep from being marked abandoned
+        }
+    }
+
     /**
      * Unregisters this pool's MBean.
      */
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 5d269febe..e80cd7e4d 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
@@ -473,14 +473,7 @@ public void returnObject(final K key, final T obj) {
                     "Returned object not currently part of this pool");
         }
 
-        synchronized(p) {
-            final PooledObjectState state = p.getState();
-            if (state != PooledObjectState.ALLOCATED) {
-                throw new IllegalStateException(
-                        "Object has already been returned to this pool or is invalid");
-            }
-            p.markReturning(); // Keep from being marked abandoned (once GKOP does this)
-        }
+        markReturningState(p);
 
         final long activeTime = p.getActiveTimeMillis();
 
@@ -492,13 +485,7 @@ public void returnObject(final K key, final T obj) {
                     } catch (final Exception e) {
                         swallowException(e);
                     }
-                    if (objectDeque.idleObjects.hasTakeWaiters()) {
-                        try {
-                            addObject(key);
-                        } catch (final Exception e) {
-                            swallowException(e);
-                        }
-                    }
+                    whenWaitersAddObject(key, objectDeque.idleObjects);
                     return;
                 }
             }
@@ -512,13 +499,7 @@ public void returnObject(final K key, final T obj) {
                 } catch (final Exception e) {
                     swallowException(e);
                 }
-                if (objectDeque.idleObjects.hasTakeWaiters()) {
-                    try {
-                        addObject(key);
-                    } catch (final Exception e) {
-                        swallowException(e);
-                    }
-                }
+                whenWaitersAddObject(key, objectDeque.idleObjects);
                 return;
             }
 
@@ -558,6 +539,20 @@ public void returnObject(final K key, final T obj) {
         }
     }
 
+    /**
+     * Whether there is at least one thread waiting on this deque, add an pool object.
+     * @param key
+     * @param idleObjects
+     */
+    private void whenWaitersAddObject(final K key, LinkedBlockingDeque<PooledObject<T>> idleObjects) {
+        if (idleObjects.hasTakeWaiters()) {
+            try {
+                addObject(key);
+            } catch (final Exception e) {
+                swallowException(e);
+            }
+        }
+    }
 
     /**
      * {@inheritDoc}
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 2928c250f..c41b5c278 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
@@ -536,14 +536,7 @@ public void returnObject(final T obj) {
             return; // Object was abandoned and removed
         }
 
-        synchronized(p) {
-            final PooledObjectState state = p.getState();
-            if (state != PooledObjectState.ALLOCATED) {
-                throw new IllegalStateException(
-                        "Object has already been returned to this pool or is invalid");
-            }
-            p.markReturning(); // Keep from being marked abandoned
-        }
+        markReturningState(p);
 
         final long activeTime = p.getActiveTimeMillis();