You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ps...@apache.org on 2012/08/31 20:02:55 UTC

svn commit: r1379533 - in /commons/proper/pool/trunk/src: changes/ main/java/org/apache/commons/pool2/impl/ test/java/org/apache/commons/pool2/impl/

Author: psteitz
Date: Fri Aug 31 18:02:54 2012
New Revision: 1379533

URL: http://svn.apache.org/viewvc?rev=1379533&view=rev
Log:
Added abandoned object removal (moved from DBCP) to GenericObjectPool.
JIRA: POOL-229

Added:
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/AbandonedConfig.java
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/TrackedUse.java
    commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java
Modified:
    commons/proper/pool/trunk/src/changes/changes.xml
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPoolMBean.java
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/PooledObject.java
    commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/PooledObjectState.java

Modified: commons/proper/pool/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/changes/changes.xml?rev=1379533&r1=1379532&r2=1379533&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/changes/changes.xml (original)
+++ commons/proper/pool/trunk/src/changes/changes.xml Fri Aug 31 18:02:54 2012
@@ -49,6 +49,9 @@ in high concurrency environments. Pools 
 objects that have been borrowed from the pool but not returned. There have been
 numerous API changes to support these and other new features as well as to
 clarify behaviour and improve consistency across the API.">
+    <action issue="POOL-229" dev="psteitz" type="update">
+      Added abandoned object removal (moved from DBCP) to GenericObjectPool.
+    </action>
     <action issue="POOL-221" dev="markt" type="fix" >
       PooledObject.state does not need to be volatile
     </action>

Added: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/AbandonedConfig.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/AbandonedConfig.java?rev=1379533&view=auto
==============================================================================
--- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/AbandonedConfig.java (added)
+++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/AbandonedConfig.java Fri Aug 31 18:02:54 2012
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.pool2.impl;
+
+import java.io.PrintWriter;
+
+/**
+ * Configuration settings for abandoned object removal.
+ *                                                                     
+ * @version $Revision:$
+ */
+public class AbandonedConfig {
+
+    /**
+     * Whether or not borrowObject performs abandoned object removal.
+     */
+    private boolean removeAbandonedOnBorrow = false;
+    
+    /**
+     * Whether or not pool maintenance (evictor) performs abandoned object
+     * removal.
+     */
+    private boolean removeAbandonedOnMaintenance = false;
+
+    /**
+     * <p>Flag to remove abandoned objects if they exceed the
+     * removeAbandonedTimeout when borrowObject is invoked.</p>
+     *
+     * <p>The default value is false.</p>
+     * 
+     * <p>If set to true, abandoned objects are removed by borrowObject if
+     * there are fewer than 2 idle objects available in the pool and
+     * <code>getNumActive() > getMaxTotal() - 3</code> </p>
+     *
+     * @return true if abandoned objects are to be removed by borrowObject
+     */
+    public boolean getRemoveAbandonedOnBorrow() {
+        return (this.removeAbandonedOnBorrow);
+    }
+
+    /**
+     * <p>Flag to remove abandoned objects if they exceed the
+     * removeAbandonedTimeout when borrowObject is invoked.</p>
+     *
+     * @param removeAbandonedOnBorrow true means abandoned objects will be
+     *   removed by borrowObject
+     * @see #getRemoveAbandonedOnBorrow()
+     */
+    public void setRemoveAbandonedOnBorrow(boolean removeAbandonedOnBorrow) {
+        this.removeAbandonedOnBorrow = removeAbandonedOnBorrow;
+    }
+    
+    /**
+     * <p>Flag to remove abandoned objects if they exceed the
+     * removeAbandonedTimeout when pool maintenance (the "evictor")
+     * runs.</p>
+     *
+     * <p>The default value is false.</p>
+     * 
+     * <p>If set to true, abandoned objects are removed by the pool
+     * maintenance thread when it runs.  This setting has no effect
+     * unless maintenance is enabled by setting 
+     *{@link GenericObjectPool#getTimeBetweenEvictionRunsMillis() timeBetweenEvictionRunsMillis}
+     * to a positive number.</p>
+     *
+     * @return true if abandoned objects are to be removed by the evictor
+     */
+    public boolean getRemoveAbandonedOnMaintenance() {
+        return (this.removeAbandonedOnMaintenance);
+    }
+
+    /**
+     * <p>Flag to remove abandoned objects if they exceed the
+     * removeAbandonedTimeout when pool maintenance runs.</p>
+     *
+     * @param removeAbandonedOnMaintenance true means abandoned objects will be
+     *   removed by pool maintenance
+     * @see #getRemoveAbandonedOnMaintenance
+     */
+    public void setRemoveAbandonedOnMaintenance(boolean removeAbandonedOnMaintenance) {
+        this.removeAbandonedOnMaintenance = removeAbandonedOnMaintenance;
+    }
+
+    /**
+     * Timeout in seconds before an abandoned object can be removed.
+     */
+    private int removeAbandonedTimeout = 300;
+
+    /** 
+     * <p>Timeout in seconds before an abandoned object can be removed.</p>
+     * 
+     * <p>The time of most recent use of an object is the maximum (latest) of
+     * {@link TrackedUse#getLastUsed()} (if this class of the object implements
+     * TrackedUse) and the time when the object was borrowed from the pool.</p>
+     * 
+     * <p>The default value is 300 seconds.</p>
+     * 
+     * @return the abandoned object timeout in seconds
+     */
+    public int getRemoveAbandonedTimeout() {
+        return (this.removeAbandonedTimeout);
+    }
+
+    /**
+     * <p>Sets the timeout in seconds before an abandoned object can be
+     * removed</p>
+     * 
+     * <p>Setting this property has no effect if 
+     * {@link #getRemoveAbandonedOnBorrow() removeAbandonedOnBorrow} and
+     * {@link #getRemoveAbandonedOnMaintenance() removeAbandonedOnMaintenance}
+     * are both false.</p>
+     *
+     * @param removeAbandonedTimeout new abandoned timeout in seconds
+     * @see #getRemoveAbandonedTimeout()
+     */
+    public void setRemoveAbandonedTimeout(int removeAbandonedTimeout) {
+        this.removeAbandonedTimeout = removeAbandonedTimeout;
+    }
+
+    /**
+     * Determines whether or not to log stack traces for application code
+     * which abandoned an object.
+     */
+    private boolean logAbandoned = false;
+
+    /**
+     * Flag to log stack traces for application code which abandoned
+     * an object.
+     *
+     * Defaults to false.
+     * Logging of abandoned objects adds overhead for every object created
+     * because a stack trace has to be generated.
+     * 
+     * @return boolean true if stack trace logging is turned on for abandoned
+     * objects
+     *
+     */
+    public boolean getLogAbandoned() {
+        return (this.logAbandoned);
+    }
+
+    /**
+     * Sets the flag to log stack traces for application code which abandoned
+     * an object.
+     * 
+     * @param logAbandoned true turns on abandoned stack trace logging
+     * @see #getLogAbandoned()
+     *
+     */
+    public void setLogAbandoned(boolean logAbandoned) {
+        this.logAbandoned = logAbandoned;
+    }
+
+    /**
+     * PrintWriter to use to log information on abandoned objects.
+     */
+    private PrintWriter logWriter = new PrintWriter(System.out);
+    
+    /**
+     * Returns the log writer being used by this configuration to log
+     * information on abandoned objects. If not set, a PrintWriter based on
+     * System.out is used.
+     *
+     * @return log writer in use
+     */
+    public PrintWriter getLogWriter() {
+        return logWriter;
+    }
+    
+    /**
+     * Sets the log writer to be used by this configuration to log
+     * information on abandoned objects.
+     * 
+     * @param logWriter The new log writer
+     */
+    public void setLogWriter(PrintWriter logWriter) {
+        this.logWriter = logWriter;
+    }
+}

Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java?rev=1379533&r1=1379532&r2=1379533&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java (original)
+++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java Fri Aug 31 18:02:54 2012
@@ -16,6 +16,8 @@
  */
 package org.apache.commons.pool2.impl;
 
+import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.concurrent.ConcurrentHashMap;
@@ -31,7 +33,7 @@ import org.apache.commons.pool2.Poolable
  * <p>
  * When coupled with the appropriate {@link PoolableObjectFactory},
  * <code>GenericObjectPool</code> provides robust pooling functionality for
- * arbitrary objects.
+ * arbitrary objects.</p>
  * <p>
  * Optionally, one may configure the pool to examine and possibly evict objects
  * as they sit idle in the pool and to ensure that a minimum number of idle
@@ -39,13 +41,25 @@ import org.apache.commons.pool2.Poolable
  * which runs asynchronously. Caution should be used when configuring this
  * optional feature. Eviction runs contend with client threads for access to
  * objects in the pool, so if they run too frequently performance issues may
- * result.
+ * result.</p>
+ * <p>
+ * The pool can also be configured to detect and remove "abandoned" objects,
+ * i.e. objects that have been checked out of the pool but neither used nor
+ * returned before the configured 
+ * {@link AbandonedConfig#getRemoveAbandonedTimeout() removeAbandonedTimeout}.
+ * Abandoned object removal can be configured to happen when
+ * <code>borrowObject</code> is invoked and the pool is close to starvation, or
+ * it can be executed by the idle object evictor, or both. If pooled objects
+ * implement the {@link TrackedUse} interface, their last use will be queried
+ * using the <code>getLastUsed</code> method on that interface; otherwise
+ * abandonment is determined by how long an object has been checked out from
+ * the pool.</p>
  * <p>
  * Implementation note: To prevent possible deadlocks, care has been taken to
  * ensure that no call to a factory method will occur within a synchronization
- * block. See POOL-125 and DBCP-44 for more information.
+ * block. See POOL-125 and DBCP-44 for more information.</p>
  * <p>
- * This class is intended to be thread-safe.
+ * This class is intended to be thread-safe.</p>
  *
  * @see GenericKeyedObjectPool
  *
@@ -90,6 +104,23 @@ public class GenericObjectPool<T> extend
 
         startEvictor(getTimeBetweenEvictionRunsMillis());
     }
+    
+    /**
+     * Create a new <code>GenericObjectPool</code> that tracks and destroys
+     * objects that are checked out, but never returned to the pool.
+     *
+     * @param config    The base pool configuration to use for this pool instance.
+     *                  The configuration is used by value. Subsequent changes to
+     *                  the configuration object will not be reflected in the
+     *                  pool.
+     * @param abandonedConfig  Configuration for abandoned object identification
+     *                         and removal.  The configuration is used by value.
+     */
+    public GenericObjectPool(PoolableObjectFactory<T> factory,
+            GenericObjectPoolConfig config, AbandonedConfig abandonedConfig) {
+        this(factory, config);
+        setAbandonedConfig(abandonedConfig);
+    }
 
     /**
      * Returns the cap on the number of "idle" instances in the pool. If maxIdle
@@ -176,10 +207,63 @@ public class GenericObjectPool<T> extend
             return minIdle;
         }
     }
+    
+    /**
+     * Whether or not abandoned object removal is configured for this pool.
+     * 
+     * @return true if this pool is configured to detect and remove
+     * abandoned objects
+     */
+    public boolean isAbandonedConfig() {
+        return abandonedConfig != null;
+    }
+    /**
+     * Returns true if abandoned object removal is configured for this pool
+     * and removal events are to be logged.
+     * 
+     * See {@link AbandonedConfig#getLogAbandoned()}
+     */
+    public boolean getLogAbandoned() {
+        return isAbandonedConfig() && abandonedConfig.getLogAbandoned();
+    }
+    
+    /**
+     * Returns true if abandoned object removal is configured to be
+     * activated by borrowObject.
+     * 
+     * See {@link AbandonedConfig#getRemoveAbandonedOnBorrow()}
+     */
+    public boolean getRemoveAbandonedOnBorrow() {
+        return isAbandonedConfig() &&
+        abandonedConfig.getRemoveAbandonedOnBorrow();
+    }
+    
+    /**
+     * Returns true if abandoned object removal is configured to be
+     * activated when the evictor runs.
+     * 
+     * See {@link AbandonedConfig#getRemoveAbandonedOnMaintenance()}
+     */
+    public boolean getRemoveAbandonedOnMaintenance() {
+        return isAbandonedConfig() &&
+        abandonedConfig.getRemoveAbandonedOnMaintenance();
+    }
+    
+    /**
+     * Returns the abandoned object timeout if abandoned object removal
+     * is configured for this pool; Integer.MAX_VALUE otherwise.
+     * 
+     * See {@link AbandonedConfig#getRemoveAbandonedTimeout()}
+     */
+    public int getRemoveAbandonedTimeout() {
+        return isAbandonedConfig() ?
+                abandonedConfig.getRemoveAbandonedTimeout() :
+                    Integer.MAX_VALUE;
+    }
 
 
     /**
-     * Sets the configuration.
+     * Sets the base pool configuration.
      *
      * @param conf the new configuration to use. This is used by value.
      *
@@ -203,6 +287,22 @@ public class GenericObjectPool<T> extend
                 conf.getSoftMinEvictableIdleTimeMillis());
         setEvictionPolicyClassName(conf.getEvictionPolicyClassName());
     }
+    
+    /**
+     * Sets the abandoned object removal configuration.
+     *
+     * @param abandonedConfig the new configuration to use. This is used by value.
+     *
+     * @see AbandonedConfig
+     */
+    public void setAbandonedConfig(AbandonedConfig abandonedConfig) throws IllegalArgumentException {
+        this.abandonedConfig = new AbandonedConfig();
+        this.abandonedConfig.setLogAbandoned(abandonedConfig.getLogAbandoned());
+        this.abandonedConfig.setLogWriter(abandonedConfig.getLogWriter());
+        this.abandonedConfig.setRemoveAbandonedOnBorrow(abandonedConfig.getRemoveAbandonedOnBorrow());
+        this.abandonedConfig.setRemoveAbandonedOnMaintenance(abandonedConfig.getRemoveAbandonedOnMaintenance());
+        this.abandonedConfig.setRemoveAbandonedTimeout(abandonedConfig.getRemoveAbandonedTimeout());
+    }
 
     /**
      * Obtain a reference to the factory used to create, destroy and validate
@@ -266,6 +366,13 @@ public class GenericObjectPool<T> extend
      */
     public T borrowObject(long borrowMaxWaitMillis) throws Exception {
         assertOpen();
+        
+        if (isAbandonedConfig() &&
+                abandonedConfig.getRemoveAbandonedOnBorrow() &&
+                (getNumIdle() < 2) &&
+                (getNumActive() > getMaxTotal() - 3) ) {
+            removeAbandoned();
+        }
 
         PooledObject<T> p = null;
 
@@ -383,12 +490,28 @@ public class GenericObjectPool<T> extend
      */
     @Override
     public void returnObject(T obj) {
-
         PooledObject<T> p = allObjects.get(obj);
-
-        if (p == null) {
-            throw new IllegalStateException(
-                    "Returned object not currently part of this pool");
+        
+        if (!isAbandonedConfig()) {
+            if (p == null) {
+                throw new IllegalStateException(
+                        "Returned object not currently part of this pool");
+            }   
+        } else {
+            if (p == null) {
+                return;  // Object was abandoned and removed
+            } else { 
+                // Make sure object is not being reclaimed
+                synchronized(p) {
+                    final PooledObjectState state = p.getState();
+                    if (state == PooledObjectState.ABANDONED ||
+                            state == PooledObjectState.INVALID) {
+                        return;
+                    } else {
+                        p.markReturning(); // Keep from being marked abandoned
+                    }
+                }
+            }  
         }
 
         long activeTime = p.getActiveTimeMillis();
@@ -420,7 +543,7 @@ public class GenericObjectPool<T> extend
 
         if (!p.deallocate()) {
             throw new IllegalStateException(
-                    "Object has already been retured to this pool");
+                    "Object has already been retured to this pool or is invalid");
         }
 
         int maxIdle = getMaxIdle();
@@ -454,9 +577,13 @@ public class GenericObjectPool<T> extend
     public void invalidateObject(T obj) throws Exception {
         PooledObject<T> p = allObjects.get(obj);
         if (p == null) {
-            throw new IllegalStateException(
-                    "Object not currently part of this pool");
-        }
+            if (isAbandonedConfig()) {
+                return;
+            } else {
+                throw new IllegalStateException(
+                        "Returned object not currently part of this pool");
+            }
+        }   
         destroy(p);
     }
 
@@ -542,93 +669,95 @@ public class GenericObjectPool<T> extend
     /**
      * {@inheritDoc}
      * <p>
-     * Successive activations of this method examine objects in in sequence,
+     * Successive activations of this method examine objects in sequence,
      * cycling through objects in oldest-to-youngest order.
      */
     @Override
     public void evict() throws Exception {
         assertOpen();
 
-        if (idleObjects.size() == 0) {
-            return;
-        }
+        if (idleObjects.size() > 0) {
+            
+            PooledObject<T> underTest = null;
+            EvictionPolicy<T> evictionPolicy = getEvictionPolicy();
+
+            synchronized (evictionLock) {
+                EvictionConfig evictionConfig = new EvictionConfig(
+                        getMinEvictableIdleTimeMillis(),
+                        getSoftMinEvictableIdleTimeMillis(),
+                        getMinIdle());
+
+                boolean testWhileIdle = getTestWhileIdle();
+
+                for (int i = 0, m = getNumTests(); i < m; i++) {
+                    if (evictionIterator == null || !evictionIterator.hasNext()) {
+                        if (getLifo()) {
+                            evictionIterator = idleObjects.descendingIterator();
+                        } else {
+                            evictionIterator = idleObjects.iterator();
+                        }
+                    }
+                    if (!evictionIterator.hasNext()) {
+                        // Pool exhausted, nothing to do here
+                        return;
+                    }
 
-        PooledObject<T> underTest = null;
-        EvictionPolicy<T> evictionPolicy = getEvictionPolicy();
+                    try {
+                        underTest = evictionIterator.next();
+                    } catch (NoSuchElementException nsee) {
+                        // Object was borrowed in another thread
+                        // Don't count this as an eviction test so reduce i;
+                        i--;
+                        evictionIterator = null;
+                        continue;
+                    }
 
-        synchronized (evictionLock) {
-            EvictionConfig evictionConfig = new EvictionConfig(
-                    getMinEvictableIdleTimeMillis(),
-                    getSoftMinEvictableIdleTimeMillis(),
-                    getMinIdle());
-
-            boolean testWhileIdle = getTestWhileIdle();
-
-            for (int i = 0, m = getNumTests(); i < m; i++) {
-                if (evictionIterator == null || !evictionIterator.hasNext()) {
-                    if (getLifo()) {
-                        evictionIterator = idleObjects.descendingIterator();
-                    } else {
-                        evictionIterator = idleObjects.iterator();
+                    if (!underTest.startEvictionTest()) {
+                        // Object was borrowed in another thread
+                        // Don't count this as an eviction test so reduce i;
+                        i--;
+                        continue;
                     }
-                }
-                if (!evictionIterator.hasNext()) {
-                    // Pool exhausted, nothing to do here
-                    return;
-                }
 
-                try {
-                    underTest = evictionIterator.next();
-                } catch (NoSuchElementException nsee) {
-                    // Object was borrowed in another thread
-                    // Don't count this as an eviction test so reduce i;
-                    i--;
-                    evictionIterator = null;
-                    continue;
-                }
-
-                if (!underTest.startEvictionTest()) {
-                    // Object was borrowed in another thread
-                    // Don't count this as an eviction test so reduce i;
-                    i--;
-                    continue;
-                }
-
-                if (evictionPolicy.evict(evictionConfig, underTest,
-                        idleObjects.size())) {
-                    destroy(underTest);
-                    destroyedByEvictorCount.incrementAndGet();
-                } else {
-                    if (testWhileIdle) {
-                        boolean active = false;
-                        try {
-                            factory.activateObject(underTest.getObject());
-                            active = true;
-                        } catch (Exception e) {
-                            destroy(underTest);
-                            destroyedByEvictorCount.incrementAndGet();
-                        }
-                        if (active) {
-                            if (!factory.validateObject(underTest.getObject())) {
+                    if (evictionPolicy.evict(evictionConfig, underTest,
+                            idleObjects.size())) {
+                        destroy(underTest);
+                        destroyedByEvictorCount.incrementAndGet();
+                    } else {
+                        if (testWhileIdle) {
+                            boolean active = false;
+                            try {
+                                factory.activateObject(underTest.getObject());
+                                active = true;
+                            } catch (Exception e) {
                                 destroy(underTest);
                                 destroyedByEvictorCount.incrementAndGet();
-                            } else {
-                                try {
-                                    factory.passivateObject(underTest.getObject());
-                                } catch (Exception e) {
+                            }
+                            if (active) {
+                                if (!factory.validateObject(underTest.getObject())) {
                                     destroy(underTest);
                                     destroyedByEvictorCount.incrementAndGet();
+                                } else {
+                                    try {
+                                        factory.passivateObject(underTest.getObject());
+                                    } catch (Exception e) {
+                                        destroy(underTest);
+                                        destroyedByEvictorCount.incrementAndGet();
+                                    }
                                 }
                             }
                         }
-                    }
-                    if (!underTest.endEvictionTest(idleObjects)) {
-                        // TODO - May need to add code here once additional
-                        // states are used
+                        if (!underTest.endEvictionTest(idleObjects)) {
+                            // TODO - May need to add code here once additional
+                            // states are used
+                        }
                     }
                 }
             }
         }
+        if (isAbandonedConfig() && abandonedConfig.getRemoveAbandonedOnMaintenance()) {
+            removeAbandoned();
+        }
     }
 
     private PooledObject<T> create() throws Exception {
@@ -648,7 +777,12 @@ public class GenericObjectPool<T> extend
             throw e;
         }
 
-        PooledObject<T> p = new PooledObject<T>(t);
+        final PooledObject p;
+        if (isAbandonedConfig() && abandonedConfig.getLogAbandoned()) {
+            p = new PooledObject<T>(t, abandonedConfig.getLogWriter());
+        } else {
+            p = new PooledObject<T>(t);
+        }
         createdCount.incrementAndGet();
         allObjects.put(t, p);
         return p;
@@ -723,7 +857,43 @@ public class GenericObjectPool<T> extend
                     Math.abs((double) numTestsPerEvictionRun)));
         }
     }
+    
+    /**
+     * Recover abandoned objects which have been checked out but
+     * not used since longer than the removeAbandonedTimeout.
+     */
+    private void removeAbandoned() {
+        // Generate a list of abandoned objects to remove
+        final long now = System.currentTimeMillis();
+        final long timeout = now - (abandonedConfig.getRemoveAbandonedTimeout() * 1000);
+        ArrayList<PooledObject<T>> remove = new ArrayList<PooledObject<T>>();
+        Iterator<PooledObject<T>> it = allObjects.values().iterator();
+        while (it.hasNext()) {
+            PooledObject<T> pooledObject = it.next();
+            synchronized (pooledObject) {
+                if (pooledObject.getState() == PooledObjectState.ALLOCATED &&
+                        pooledObject.getLastUsed() <= timeout) {
+                    pooledObject.markAbandoned();
+                    remove.add(pooledObject);
+                }
+            }
+        }
 
+        // Now remove the abandoned objects
+        Iterator<PooledObject<T>> itr = remove.iterator();
+        while (itr.hasNext()) {
+            PooledObject<T> pooledObject = itr.next();
+            if (abandonedConfig.getLogAbandoned()) {
+                pooledObject.printStackTrace();
+            }             
+            try {
+                invalidateObject(pooledObject.getObject());
+            } catch (Exception e) {
+                e.printStackTrace();
+            }
+        } 
+    }
+    
     //--- JMX support ----------------------------------------------------------
 
     /**
@@ -754,7 +924,8 @@ public class GenericObjectPool<T> extend
      * All of the objects currently associated with this pool in any state. It
      * excludes objects that have been destroyed. The size of
      * {@link #allObjects} will always be less than or equal to {@link
-     * #_maxActive}.
+     * #_maxActive}. Map keys are pooled objects, values are the PooledObject
+     * wrappers used internally by the pool.
      */
     private final Map<T, PooledObject<T>> allObjects =
         new ConcurrentHashMap<T, PooledObject<T>>();
@@ -772,4 +943,7 @@ public class GenericObjectPool<T> extend
     // JMX specific attributes
     private static final String ONAME_BASE =
         "org.apache.commoms.pool2:type=GenericObjectPool,name=";
+    
+    // Additional configuration properties for abandoned object tracking
+    private volatile AbandonedConfig abandonedConfig = null;
 }

Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPoolMBean.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPoolMBean.java?rev=1379533&r1=1379532&r2=1379533&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPoolMBean.java (original)
+++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPoolMBean.java Fri Aug 31 18:02:54 2012
@@ -24,7 +24,7 @@ package org.apache.commons.pool2.impl;
  * @since 2.0
  */
 public interface GenericObjectPoolMBean {
-    // Getters for configuration settings
+    // Getters for basic configuration settings
     /**
      * See {@link GenericObjectPool#getBlockWhenExhausted()}
      */
@@ -138,4 +138,26 @@ public interface GenericObjectPoolMBean 
      * See {@link GenericObjectPool#getNumWaiters()}
      */
     int getNumWaiters();
+    
+    // Getters for abandoned object removal configuration
+    /**
+     * See {@link GenericObjectPool#isAbandonedConfig()}
+     */
+    boolean isAbandonedConfig();  
+    /**
+     * See {@link GenericObjectPool#getLogAbandoned()}
+     */
+    boolean getLogAbandoned();
+    /**
+     * See {@link GenericObjectPool#getRemoveAbandonedOnBorrow()}
+     */
+    boolean getRemoveAbandonedOnBorrow();
+    /**
+     * See {@link GenericObjectPool#getRemoveAbandonedOnMaintenance()}
+     */
+    boolean getRemoveAbandonedOnMaintenance();
+    /**
+     * See {@link GenericObjectPool#getRemoveAbandonedTimeout()}
+     */
+    int getRemoveAbandonedTimeout();
 }

Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/PooledObject.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/PooledObject.java?rev=1379533&r1=1379532&r2=1379533&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/PooledObject.java (original)
+++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/PooledObject.java Fri Aug 31 18:02:54 2012
@@ -16,6 +16,10 @@
  */
 package org.apache.commons.pool2.impl;
 
+import java.io.PrintWriter;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+
 /**
  * This wrapper is used to track the additional information, such as state, for
  * the pooled objects.
@@ -35,9 +39,19 @@ public class PooledObject<T> implements 
     private final long createTime = System.currentTimeMillis();
     private volatile long lastBorrowTime = createTime;
     private volatile long lastReturnTime = createTime;
+    private final Exception createdBy;
+    private final PrintWriter logWriter;
 
     public PooledObject(T object) {
         this.object = object;
+        createdBy = null;
+        logWriter = null;
+    }
+    
+    public PooledObject(T object, PrintWriter logWriter) {
+        this.object = object;
+        this.logWriter = logWriter;
+        createdBy = new AbandonedObjectException();
     }
 
     /**
@@ -89,6 +103,23 @@ public class PooledObject<T> implements 
     public long getLastReturnTime() {
         return lastReturnTime;
     }
+    
+    /**
+     * Return an estimate of the last time this object was used.  If the class
+     * of the pooled object implements {@link TrackedUse}, what is returned is 
+     * the maximum of {@link TrackedUse#getLastUsed()} and
+     * {@link #getLastBorrowTime()}; otherwise this method gives the same
+     * value as {@link #getLastBorrowTime()}.
+     * 
+     * @return the last time this object was used
+     */
+    public long getLastUsed() {
+        if (object instanceof TrackedUse) {
+            return Math.max(((TrackedUse) object).getLastUsed(), lastBorrowTime);
+        } else {
+            return lastBorrowTime;
+        }
+    }
 
     /**
      * Orders instances based on idle time - i.e. the length of time since the
@@ -194,7 +225,8 @@ public class PooledObject<T> implements 
      * @return {@code true} if the state was {@link PooledObjectState#ALLOCATED ALLOCATED}
      */
     public synchronized boolean deallocate() {
-        if (state == PooledObjectState.ALLOCATED) {
+        if (state == PooledObjectState.ALLOCATED ||
+                state == PooledObjectState.RETURNING) {
             state = PooledObjectState.IDLE;
             lastReturnTime = System.currentTimeMillis();
             return true;
@@ -209,4 +241,65 @@ public class PooledObject<T> implements 
     public synchronized void invalidate() {
         state = PooledObjectState.INVALID;
     }
+    
+    /**
+     * Prints the stack trace of the code that created this pooled object to
+     * the configured log writer.  Does nothing of no PrintWriter was supplied
+     * to the constructor.
+     */
+    public void printStackTrace() {
+        if (createdBy != null && logWriter != null) {
+            createdBy.printStackTrace(logWriter);
+        }
+    }
+    
+    /**
+     * Returns the state of this object.
+     * @return state
+     */
+    public synchronized PooledObjectState getState() {
+        return state;
+    }
+    
+    /**
+     * Marks the pooled object as abandoned.
+     */
+    public synchronized void markAbandoned() {
+        state = PooledObjectState.ABANDONED;
+    }
+    
+    /**
+     * Marks the object as returning to the pool.
+     */
+    public synchronized void markReturning() {
+        state = PooledObjectState.RETURNING;
+    }
+    
+    static class AbandonedObjectException extends Exception {
+
+        private static final long serialVersionUID = 7398692158058772916L;
+
+        /** Date format */
+        //@GuardedBy("this")
+        private static final SimpleDateFormat format = new SimpleDateFormat
+            ("'Pooled object created' yyyy-MM-dd HH:mm:ss " +
+             "'by the following code was never returned to the pool:'");
+
+        private final long _createdTime;
+
+        public AbandonedObjectException() {
+            _createdTime = System.currentTimeMillis();
+        }
+
+        // Override getMessage to avoid creating objects and formatting
+        // dates unless the log message will actually be used.
+        @Override
+        public String getMessage() {
+            String msg;
+            synchronized(format) {
+                msg = format.format(new Date(_createdTime));
+            }
+            return msg;
+        }
+    }
 }

Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/PooledObjectState.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/PooledObjectState.java?rev=1379533&r1=1379532&r2=1379533&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/PooledObjectState.java (original)
+++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/PooledObjectState.java Fri Aug 31 18:02:54 2012
@@ -76,5 +76,15 @@ public enum PooledObjectState {
      * Failed maintenance (e.g. eviction test or validation) and will be / has
      * been destroyed
      */
-    INVALID
+    INVALID,
+    
+    /**
+     * Deemed abandoned, to be invalidated.
+     */
+    ABANDONED,
+    
+    /**
+     * Returning to the pool.
+     */
+    RETURNING
 }

Added: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/TrackedUse.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/TrackedUse.java?rev=1379533&view=auto
==============================================================================
--- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/TrackedUse.java (added)
+++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/TrackedUse.java Fri Aug 31 18:02:54 2012
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.pool2.impl;
+
+/**
+ * Methods to support usage tracking for instances managed by pools configured
+ * to remove abandoned objects.
+ * 
+ * @version $Revision:$
+ */
+public interface TrackedUse {
+
+    /**
+     * Get the last time this object was used in ms.
+     *
+     * @return long time in ms
+     */
+    long getLastUsed();
+}

Added: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java?rev=1379533&view=auto
==============================================================================
--- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java (added)
+++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java Fri Aug 31 18:02:54 2012
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.pool2.impl;
+
+import java.lang.management.ManagementFactory;
+import java.util.ArrayList;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.management.MBeanServer;
+import javax.management.ObjectName;
+
+import junit.framework.TestCase;
+
+import org.apache.commons.pool2.PoolableObjectFactory;
+import org.junit.Assert;
+
+/**
+ * TestCase for AbandonedObjectPool
+ * 
+ * @version $Revision: 1158659 $ $Date: 2011-08-17 05:37:26 -0700 (Wed, 17 Aug 2011) $
+ */
+public class TestAbandonedObjectPool extends TestCase {
+    private GenericObjectPool<PooledTestObject> pool = null;
+    private AbandonedConfig abandonedConfig = null;
+
+    @Override
+    public void setUp() throws Exception {
+        abandonedConfig = new AbandonedConfig();
+        
+        // -- Uncomment the following line to enable logging -- 
+        // abandonedConfig.setLogAbandoned(true);
+        
+        abandonedConfig.setRemoveAbandonedOnBorrow(true);
+        abandonedConfig.setRemoveAbandonedTimeout(1);
+        pool = new GenericObjectPool<PooledTestObject>(
+               new SimpleFactory(), new GenericObjectPoolConfig(), abandonedConfig);
+    }
+
+    @Override
+    public void tearDown() throws Exception {
+        String poolName = pool.getJmxName().toString();
+        pool.clear();
+        pool.close();
+        pool = null;
+
+        MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
+        Set<ObjectName> result = mbs.queryNames(new ObjectName(
+                "org.apache.commoms.pool2:type=GenericObjectPool,*"), null);
+        // There should be no registered pools at this point
+        int registeredPoolCount = result.size();
+        StringBuilder msg = new StringBuilder("Current pool is: ");
+        msg.append(poolName);
+        msg.append("  Still open pools are: ");
+        for (ObjectName name : result) {
+            // Clean these up ready for the next test
+            msg.append(name.toString());
+            msg.append(" created via\n");
+            msg.append(mbs.getAttribute(name, "CreationStackTrace"));
+            msg.append('\n');
+            mbs.unregisterMBean(name);
+        }
+        Assert.assertEquals(msg.toString(), 0, registeredPoolCount);
+    }
+
+    /**
+    * Tests fix for Bug 28579, a bug in AbandonedObjectPool that causes numActive to go negative
+    * in GenericObjectPool
+    * 
+    */
+    public void testConcurrentInvalidation() throws Exception {
+        final int POOL_SIZE = 30;
+        pool.setMaxTotal(POOL_SIZE);
+        pool.setMaxIdle(POOL_SIZE);
+        pool.setBlockWhenExhausted(false);
+
+        // Exhaust the connection pool
+        ArrayList<PooledTestObject> vec = new ArrayList<PooledTestObject>();
+        for (int i = 0; i < POOL_SIZE; i++) {
+            vec.add(pool.borrowObject());
+        }
+        
+        // Abandon all borrowed objects
+        for (int i = 0; i < vec.size(); i++) {
+            ((PooledTestObject)vec.get(i)).setAbandoned(true);
+        }
+
+        // Try launching a bunch of borrows concurrently.  Abandoned sweep will be triggered for each.
+        final int CONCURRENT_BORROWS = 5;
+        Thread[] threads = new Thread[CONCURRENT_BORROWS];
+        for (int i = 0; i < CONCURRENT_BORROWS; i++) {
+            threads[i] = new ConcurrentBorrower(vec);
+            threads[i].start();
+        }
+
+        // Wait for all the threads to finish
+        for (int i = 0; i < CONCURRENT_BORROWS; i++) {
+            threads[i].join();
+        }
+        
+        // Return all objects that have not been destroyed
+        for (int i = 0; i < vec.size(); i++) {
+            PooledTestObject pto = (PooledTestObject)vec.get(i);
+            if (pto.isActive()) {
+                pool.returnObject(pto);
+            }
+        }
+        
+        // Now, the number of active instances should be 0
+        assertTrue("numActive should have been 0, was " + pool.getNumActive(), pool.getNumActive() == 0);
+    }
+    
+    /**
+     * Verify that an object that gets flagged as abandoned and is subsequently returned
+     * is destroyed instead of being returned to the pool (and possibly later destroyed
+     * inappropriately).
+     */
+    public void testAbandonedReturn() throws Exception {
+        abandonedConfig = new AbandonedConfig(); 
+        abandonedConfig.setRemoveAbandonedOnBorrow(true);
+        abandonedConfig.setRemoveAbandonedTimeout(1);
+        pool.close();  // Unregister pool created by setup
+        pool = new GenericObjectPool<PooledTestObject>(
+                new SimpleFactory(200, 0),
+                new GenericObjectPoolConfig(), abandonedConfig);
+        final int n = 10;
+        pool.setMaxTotal(n);
+        pool.setBlockWhenExhausted(false);
+        PooledTestObject obj = null;
+        for (int i = 0; i < n - 2; i++) {
+            obj = pool.borrowObject();
+        }
+        final int deadMansHash = obj.hashCode();
+        ConcurrentReturner returner = new ConcurrentReturner(obj);
+        Thread.sleep(2000);  // abandon checked out instances
+        // Now start a race - returner waits until borrowObject has kicked
+        // off removeAbandoned and then returns an instance that borrowObject
+        // will deem abandoned.  Make sure it is not returned to the borrower.
+        returner.start();    // short delay, then return instance
+        assertTrue(pool.borrowObject().hashCode() != deadMansHash);
+        assertEquals(0, pool.getNumIdle());
+        assertEquals(1, pool.getNumActive());
+    }
+    
+    /**
+     * Verify that an object that gets flagged as abandoned and is subsequently
+     * invalidated is only destroyed (and pool counter decremented) once.
+     */
+    public void testAbandonedInvalidate() throws Exception {
+        abandonedConfig = new AbandonedConfig(); 
+        abandonedConfig.setRemoveAbandonedOnMaintenance(true);
+        abandonedConfig.setRemoveAbandonedTimeout(1);
+        pool.close();  // Unregister pool created by setup
+        pool = new GenericObjectPool<PooledTestObject>(
+                new SimpleFactory(200, 0),  // destroys take 200 ms
+                new GenericObjectPoolConfig(), abandonedConfig);
+        final int n = 10;
+        pool.setMaxTotal(n);
+        pool.setBlockWhenExhausted(false);
+        pool.setTimeBetweenEvictionRunsMillis(500);
+        PooledTestObject obj = null;
+        for (int i = 0; i < 5; i++) {
+            obj = pool.borrowObject();
+        } 
+        Thread.sleep(1000);          // abandon checked out instances and let evictor start
+        pool.invalidateObject(obj);  // Should not trigger another destroy / decrement
+        Thread.sleep(2000);          // give evictor time to finish destroys
+        assertEquals(0, pool.getNumActive());
+        assertEquals(5, pool.getDestroyedCount());
+    }
+    
+    /**
+     * Verify that an object that the evictor identifies as abandoned while it
+     * is in process of being returned to the pool is not destroyed.
+     */
+    public void testRemoveAbandonedWhileReturning() throws Exception {
+        abandonedConfig = new AbandonedConfig(); 
+        abandonedConfig.setRemoveAbandonedOnMaintenance(true);
+        abandonedConfig.setRemoveAbandonedTimeout(1);
+        pool.close();  // Unregister pool created by setup
+        pool = new GenericObjectPool<PooledTestObject>(
+                new SimpleFactory(0, 1000),  // validate takes 1 second
+                new GenericObjectPoolConfig(), abandonedConfig);
+        final int n = 10;
+        pool.setMaxTotal(n);
+        pool.setBlockWhenExhausted(false);
+        pool.setTimeBetweenEvictionRunsMillis(500);
+        pool.setTestOnReturn(true);
+        // Borrow an object, wait long enough for it to be abandoned
+        // then arrange for evictor to run while it is being returned
+        // validation takes a second, evictor runs every 500 ms
+        PooledTestObject obj = pool.borrowObject();
+        Thread.sleep(50);       // abandon obj
+        pool.returnObject(obj); // evictor will run during validation
+        PooledTestObject obj2 = pool.borrowObject();
+        assertEquals(obj, obj2);          // should get original back
+        assertTrue(!obj2.isDestroyed());  // and not destroyed 
+    }
+    
+    class ConcurrentBorrower extends Thread {
+        private ArrayList<PooledTestObject> _borrowed;
+        
+        public ConcurrentBorrower(ArrayList<PooledTestObject> borrowed) {
+            _borrowed = borrowed;
+        }
+        
+        @Override
+        public void run() {
+            try {
+                _borrowed.add(pool.borrowObject());
+            } catch (Exception e) {
+                // expected in most cases
+            }
+        }
+    }
+    
+    class ConcurrentReturner extends Thread {
+        private PooledTestObject returned;
+        public ConcurrentReturner(PooledTestObject obj) {
+            returned = obj;
+        }
+        @Override
+        public void run() {
+            try {
+                sleep(20);
+                pool.returnObject(returned);
+            } catch (Exception e) {
+                // ignore
+            }
+        }
+    }
+    
+    class SimpleFactory implements PoolableObjectFactory<PooledTestObject> {
+        
+        private final long destroyLatency;
+        private final long validateLatency;
+        
+        public SimpleFactory() {
+            destroyLatency = 0;
+            validateLatency = 0;
+        }
+        
+        public SimpleFactory(long destroyLatency, long validateLatency) {
+            this.destroyLatency = destroyLatency;
+            this.validateLatency = validateLatency;
+        }
+
+        public PooledTestObject makeObject() {
+            return new PooledTestObject(abandonedConfig);
+        }
+        
+        public boolean validateObject(PooledTestObject obj) {
+            try {
+                Thread.sleep(validateLatency);
+            } catch (Exception ex) {
+                // ignore
+            }
+            return true;
+        }
+        
+        public void activateObject(PooledTestObject obj) {
+            ((PooledTestObject)obj).setActive(true);
+        }
+        
+        public void passivateObject(PooledTestObject obj) {
+            ((PooledTestObject)obj).setActive(false);
+        }
+
+        public void destroyObject(PooledTestObject obj) throws Exception {
+            obj.setActive(false);
+            // while destroying instances, yield control to other threads
+            // helps simulate threading errors
+            Thread.yield();
+            if (destroyLatency != 0) {
+                Thread.sleep(destroyLatency);
+            }
+            obj.destroy();
+        }
+    }
+}
+
+class PooledTestObject implements TrackedUse {
+    private boolean active = false;
+    private boolean destroyed = false;
+    private int _hash = 0;
+    private boolean _abandoned = false;
+    private static AtomicInteger hash = new AtomicInteger();
+    
+    public PooledTestObject(AbandonedConfig config) {
+        _hash = hash.incrementAndGet();
+    }
+    
+    public synchronized void setActive(boolean b) {
+        active = b;
+    }
+
+    public synchronized boolean isActive() {
+        return active;
+    }
+    
+    public void destroy() {
+        destroyed = true;
+    }
+    
+    public boolean isDestroyed() {
+        return destroyed;
+    }
+    
+    @Override
+    public int hashCode() {
+        return _hash;
+    }
+    
+    public void setAbandoned(boolean b) {
+        _abandoned = b;
+    }
+    
+    public long getLastUsed() {
+        if (_abandoned) {
+            // Abandoned object sweep will occur no matter what the value of removeAbandonedTimeout,
+            // because this indicates that this object was last used decades ago
+            return 1;
+        } else {
+            // Abandoned object sweep won't clean up this object
+            return 0;
+        }
+    }
+    
+    @Override
+    public boolean equals(Object obj) {
+        if (!(obj instanceof PooledTestObject)) return false;
+        return obj.hashCode() == hashCode();
+    }
+}
+