Author: psteitz
Date: Sun Apr 11 18:01:51 2010
New Revision: 932962
URL: http://svn.apache.org/viewvc?rev=932962&view=rev
Log:
Ensured that when factories are changed, idle instances are destroyed using the
original factories. Also deprecated the factory setters. JIRA: POOL-157.
Reported by David Hu.
Modified:
commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
commons/proper/pool/trunk/xdocs/changes.xml
Modified:
commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
URL:
http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java?rev=932962&r1=932961&r2=932962&view=diff
==============================================================================
---
commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
(original)
+++
commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
Sun Apr 11 18:01:51 2010
@@ -1354,7 +1354,7 @@ public class GenericKeyedObjectPool exte
pool.queue.clear();
}
}
- destroy(toDestroy);
+ destroy(toDestroy, _factory);
}
/**
@@ -1415,7 +1415,7 @@ public class GenericKeyedObjectPool exte
}
}
- destroy(toDestroy);
+ destroy(toDestroy, _factory);
}
/**
@@ -1444,22 +1444,23 @@ public class GenericKeyedObjectPool exte
_totalInternalProcessing + pool.queue.size();
pool.queue.clear();
}
- destroy(toDestroy);
+ destroy(toDestroy, _factory);
}
/**
* Assuming Map<Object,Collection<ObjectTimestampPair>>, destroy all
- * ObjectTimestampPair.value
+ * ObjectTimestampPair.value using the supplied factory.
*
* @param m Map containing keyed pools to clear
+ * @param factory KeyedPoolableObjectFactory used to destroy the objects
*/
- private void destroy(Map m) {
+ private void destroy(Map m, KeyedPoolableObjectFactory factory) {
for (Iterator keys = m.keySet().iterator(); keys.hasNext();) {
Object key = keys.next();
Collection c = (Collection) m.get(key);
for (Iterator it = c.iterator(); it.hasNext();) {
try {
- _factory.destroyObject(
+ factory.destroyObject(
key,((ObjectTimestampPair)(it.next())).value);
} catch(Exception e) {
// ignore error, keep destroying the rest
@@ -1753,13 +1754,15 @@ public class GenericKeyedObjectPool exte
*
* <p>If this method is called when objects are checked out of any of the
keyed pools,
* an IllegalStateException is thrown. Calling this method also has the
side effect of
- * destroying any idle instances in existing keyed pools.</p>
+ * destroying any idle instances in existing keyed pools, using the
original factory.</p>
*
* @param factory KeyedPoolableObjectFactory to use when creating keyed
object pool instances
* @throws IllegalStateException if there are active (checked out)
instances associated with this keyed object pool
+ * @deprecated to be removed in version 2.0
*/
public void setFactory(KeyedPoolableObjectFactory factory) throws
IllegalStateException {
Map toDestroy = new HashMap();
+ final KeyedPoolableObjectFactory oldFactory = _factory;
synchronized (this) {
assertOpen();
if (0 < getNumActive()) {
@@ -1785,7 +1788,7 @@ public class GenericKeyedObjectPool exte
_factory = factory;
}
}
- destroy(toDestroy);
+ destroy(toDestroy, oldFactory);
}
/**
Modified:
commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
URL:
http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java?rev=932962&r1=932961&r2=932962&view=diff
==============================================================================
---
commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
(original)
+++
commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
Sun Apr 11 18:01:51 2010
@@ -1289,19 +1289,22 @@ public class GenericObjectPool extends B
_numInternalProcessing = _numInternalProcessing + _pool._size;
_pool.clear();
}
- destroy(toDestroy);
+ destroy(toDestroy, _factory);
}
/**
- * Private method to destroy all the objects in a collection. Assumes
- * objects in the collection are instances of ObjectTimestampPair
+ * Private method to destroy all the objects in a collection using the
+ * supplied object factory. Assumes that objects in the collection are
+ * instances of ObjectTimestampPair and that the object instances that
+ * they wrap were created by the factory.
*
* @param c Collection of objects to destroy
+ * @param factory PoolableConnectionFactory used to destroy the objects
*/
- private void destroy(Collection c) {
+ private void destroy(Collection c, PoolableObjectFactory factory) {
for (Iterator it = c.iterator(); it.hasNext();) {
try {
-
_factory.destroyObject(((ObjectTimestampPair)(it.next())).value);
+
factory.destroyObject(((ObjectTimestampPair)(it.next())).value);
} catch(Exception e) {
// ignore error, keep destroying the rest
} finally {
@@ -1458,13 +1461,17 @@ public class GenericObjectPool extends B
* Sets the {...@link PoolableObjectFactory factory} this pool uses
* to create new instances. Trying to change
* the <code>factory</code> while there are borrowed objects will
- * throw an {...@link IllegalStateException}.
+ * throw an {...@link IllegalStateException}. If there are instances idle
+ * in the pool when this method is invoked, these will be destroyed
+ * using the original factory.
*
* @param factory the {...@link PoolableObjectFactory} used to create new
instances.
* @throws IllegalStateException when the factory cannot be set at this
time
+ * @deprecated to be removed in version 2.0
*/
public void setFactory(PoolableObjectFactory factory) throws
IllegalStateException {
List toDestroy = new ArrayList();
+ final PoolableObjectFactory oldFactory = _factory;
synchronized (this) {
assertOpen();
if(0 < getNumActive()) {
@@ -1476,7 +1483,7 @@ public class GenericObjectPool extends B
}
_factory = factory;
}
- destroy(toDestroy);
+ destroy(toDestroy, oldFactory);
}
/**
Modified: commons/proper/pool/trunk/xdocs/changes.xml
URL:
http://svn.apache.org/viewvc/commons/proper/pool/trunk/xdocs/changes.xml?rev=932962&r1=932961&r2=932962&view=diff
==============================================================================
--- commons/proper/pool/trunk/xdocs/changes.xml (original)
+++ commons/proper/pool/trunk/xdocs/changes.xml Sun Apr 11 18:01:51 2010
@@ -21,6 +21,19 @@
</properties>
<body>
<release version="1.5.5" date="TBD" description="TBD">
+ <action dev="psteitz" type="fix" issue="POOL-157" due-to="David Hu">
+ GenericObjectPool and GenericKeyedObjectPool setFactory methods destroy
idle instances
+ in the pool by contract. Prior to the fix for this issue, newly set
factories were being
+ used to destroy idle instances, rather than the factories used to create
them. The
+ setFactory methods have also been deprecated, to be removed in version
2.0.
+ </action>
+ <action dev="sebb" type="update" issue="POOL-156">
+ ObjectPool classes can ignore Throwable. Added consistent handling for
Throwables
+ that are normally swallowed including always re-throwing certain
Throwables (e.g. ThreadDeath).
+ </action>
+ <action dev="markt" type="fix" issue="POOL-162">
+ When waiting threads are interrupted, GOP, GKOP may leak capacity.
+ </action>
<action dev="psteitz" type="fix" issue="POOL-154" due-to="Glen Mazza">
Documentation for the close method in GenericObjectPool and
GenericKeyedObjectPool
incorrectly states that this method does not clear the pool.
