You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ma...@apache.org on 2012/02/14 00:28:45 UTC

svn commit: r1243741 - in /commons/proper/pool/branches/1_5_RELEASE: ./ src/changes/ src/java/org/apache/commons/pool/impl/ src/test/org/apache/commons/pool/impl/

Author: markt
Date: Mon Feb 13 23:28:44 2012
New Revision: 1243741

URL: http://svn.apache.org/viewvc?rev=1243741&view=rev
Log:
Fix POOL-161. Patch provided by Sylvain Laurent.
Ensure the evictor thread runs with the same class loader that was in use when the pool was created to ensure that the factory class is visible.

Added:
    commons/proper/pool/branches/1_5_RELEASE/src/test/org/apache/commons/pool/impl/TestGenericObjectPoolClassLoaders.java
      - copied, changed from r1243737, commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericObjectPoolClassLoaders.java
    commons/proper/pool/branches/1_5_RELEASE/src/test/org/apache/commons/pool/impl/test1
      - copied unchanged from r1243737, commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/test1
    commons/proper/pool/branches/1_5_RELEASE/src/test/org/apache/commons/pool/impl/test2
      - copied unchanged from r1243737, commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/test2
Modified:
    commons/proper/pool/branches/1_5_RELEASE/   (props changed)
    commons/proper/pool/branches/1_5_RELEASE/src/changes/changes.xml
    commons/proper/pool/branches/1_5_RELEASE/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
    commons/proper/pool/branches/1_5_RELEASE/src/java/org/apache/commons/pool/impl/GenericObjectPool.java

Propchange: commons/proper/pool/branches/1_5_RELEASE/
------------------------------------------------------------------------------
    svn:mergeinfo = /commons/proper/pool/branches/POOL_1_X:1243737

Modified: commons/proper/pool/branches/1_5_RELEASE/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/pool/branches/1_5_RELEASE/src/changes/changes.xml?rev=1243741&r1=1243740&r2=1243741&view=diff
==============================================================================
--- commons/proper/pool/branches/1_5_RELEASE/src/changes/changes.xml (original)
+++ commons/proper/pool/branches/1_5_RELEASE/src/changes/changes.xml Mon Feb 13 23:28:44 2012
@@ -20,6 +20,12 @@
     <title>Commons Pool Changes</title>
   </properties>
   <body>
+  <release version="1.5.8" date="?" description="This is a patch release, including bugfixes only.">
+    <action dev="markt" type="fix" issue="POOL-161" due-to="Sylvain Laurent">
+      Ensure the evictor thread runs with the same class loader that was in use
+      when the pool was created to ensure that the factory class is visible.
+    </action>
+  </release>
   <release version="1.5.7" date="2011-12-20" description="This is a patch release, including bugfixes only.">
     <action dev="psteitz" type="fix" issue="POOL-189" due-to="Bill Speirs">
       Awaken threads waiting on borrowObject when a pool has been closed and have them throw

Modified: commons/proper/pool/branches/1_5_RELEASE/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/branches/1_5_RELEASE/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java?rev=1243741&r1=1243740&r2=1243741&view=diff
==============================================================================
--- commons/proper/pool/branches/1_5_RELEASE/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java (original)
+++ commons/proper/pool/branches/1_5_RELEASE/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java Mon Feb 13 23:28:44 2012
@@ -606,6 +606,8 @@ public class GenericKeyedObjectPool exte
             long timeBetweenEvictionRunsMillis, int numTestsPerEvictionRun, long minEvictableIdleTimeMillis,
             boolean testWhileIdle, boolean lifo) {
         _factory = factory;
+        // save the current CCL to be used later by the evictor Thread
+        _factoryClassLoader = Thread.currentThread().getContextClassLoader();
         _maxActive = maxActive;
         _lifo = lifo;
         switch (whenExhaustedAction) {
@@ -1849,6 +1851,8 @@ public class GenericKeyedObjectPool exte
                     }
                 }
                 _factory = factory;
+                // save the current CCL to be used later by the evictor Thread
+                _factoryClassLoader = Thread.currentThread().getContextClassLoader();
             }
         }
         destroy(toDestroy, oldFactory);
@@ -2358,23 +2362,35 @@ public class GenericKeyedObjectPool exte
         /**
          * Run pool maintenance.  Evict objects qualifying for eviction and then
          * invoke {@link GenericKeyedObjectPool#ensureMinIdle()}.
+         * Since the Timer that invokes Evictors is shared for all Pools, we try
+         * to restore the ContextClassLoader that created the pool.
          */
         public void run() {
-            //Evict from the pool
+            ClassLoader savedClassLoader =
+                    Thread.currentThread().getContextClassLoader();
             try {
-                evict();
-            } catch(Exception e) {
-                // ignored
-            } catch(OutOfMemoryError oome) {
-                // Log problem but give evictor thread a chance to continue in
-                // case error is recoverable
-                oome.printStackTrace(System.err);
-            }
-            //Re-create idle instances.
-            try {
-                ensureMinIdle();
-            } catch (Exception e) {
-                // ignored
+                //set the classloader for the factory
+                Thread.currentThread().setContextClassLoader(
+                        _factoryClassLoader);
+                //Evict from the pool
+                try {
+                    evict();
+                } catch(Exception e) {
+                    // ignored
+                } catch(OutOfMemoryError oome) {
+                    // Log problem but give evictor thread a chance to continue in
+                    // case error is recoverable
+                    oome.printStackTrace(System.err);
+                }
+                //Re-create idle instances.
+                try {
+                    ensureMinIdle();
+                } catch (Exception e) {
+                    // ignored
+                }
+            } finally {
+                //restore the previous CCL
+                Thread.currentThread().setContextClassLoader(savedClassLoader);
             }
         }
     }
@@ -2698,6 +2714,14 @@ public class GenericKeyedObjectPool exte
     private KeyedPoolableObjectFactory _factory = null;
 
     /**
+     * Class loader for evictor thread to use since in a J2EE or similar
+     * environment the context class loader for the evictor thread may have
+     * visibility of the correct factory. See POOL-161.
+     */
+    private ClassLoader _factoryClassLoader = null;
+
+
+    /**
      * My idle object eviction {@link TimerTask}, if any.
      */
     private Evictor _evictor = null;

Modified: commons/proper/pool/branches/1_5_RELEASE/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/branches/1_5_RELEASE/src/java/org/apache/commons/pool/impl/GenericObjectPool.java?rev=1243741&r1=1243740&r2=1243741&view=diff
==============================================================================
--- commons/proper/pool/branches/1_5_RELEASE/src/java/org/apache/commons/pool/impl/GenericObjectPool.java (original)
+++ commons/proper/pool/branches/1_5_RELEASE/src/java/org/apache/commons/pool/impl/GenericObjectPool.java Mon Feb 13 23:28:44 2012
@@ -571,6 +571,8 @@ public class GenericObjectPool extends B
             int numTestsPerEvictionRun, long minEvictableIdleTimeMillis, boolean testWhileIdle,
             long softMinEvictableIdleTimeMillis, boolean lifo) {
         _factory = factory;
+        // save the current CCL to be used later by the evictor Thread
+        _factoryClassLoader = Thread.currentThread().getContextClassLoader();
         _maxActive = maxActive;
         _lifo = lifo;
         switch(whenExhaustedAction) {
@@ -1522,6 +1524,8 @@ public class GenericObjectPool extends B
                 _pool.clear();
             }
             _factory = factory;
+            // save the current CCL to be used later by the evictor Thread
+            _factoryClassLoader = Thread.currentThread().getContextClassLoader();
         }
         destroy(toDestroy, oldFactory); 
     }
@@ -1756,21 +1760,33 @@ public class GenericObjectPool extends B
         /**
          * Run pool maintenance.  Evict objects qualifying for eviction and then
          * invoke {@link GenericObjectPool#ensureMinIdle()}.
+         * Since the Timer that invokes Evictors is shared for all Pools, we try
+         * to restore the ContextClassLoader that created the pool.
          */
         public void run() {
+            ClassLoader savedClassLoader =
+                    Thread.currentThread().getContextClassLoader();
             try {
-                evict();
-            } catch(Exception e) {
-                // ignored
-            } catch(OutOfMemoryError oome) {
-                // Log problem but give evictor thread a chance to continue in
-                // case error is recoverable
-                oome.printStackTrace(System.err);
-            }
-            try {
-                ensureMinIdle();
-            } catch(Exception e) {
-                // ignored
+                //set the classloader for the factory
+                Thread.currentThread().setContextClassLoader(
+                        _factoryClassLoader);
+                try {
+                    evict();
+                } catch(Exception e) {
+                    // ignored
+                } catch(OutOfMemoryError oome) {
+                    // Log problem but give evictor thread a chance to continue in
+                    // case error is recoverable
+                    oome.printStackTrace(System.err);
+                }
+                try {
+                    ensureMinIdle();
+                } catch(Exception e) {
+                    // ignored
+                }
+            } finally {
+                //restore the previous CCL
+                Thread.currentThread().setContextClassLoader(savedClassLoader);
             }
         }
     }
@@ -2053,6 +2069,13 @@ public class GenericObjectPool extends B
     private PoolableObjectFactory _factory = null;
 
     /**
+     * Class loader for evictor thread to use since in a J2EE or similar
+     * environment the context class loader for the evictor thread may have
+     * visibility of the correct factory. See POOL-161.
+     */
+    private ClassLoader _factoryClassLoader = null;
+
+    /**
      * The number of objects {@link #borrowObject} borrowed
      * from the pool, but not yet returned.
      */

Copied: commons/proper/pool/branches/1_5_RELEASE/src/test/org/apache/commons/pool/impl/TestGenericObjectPoolClassLoaders.java (from r1243737, commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericObjectPoolClassLoaders.java)
URL: http://svn.apache.org/viewvc/commons/proper/pool/branches/1_5_RELEASE/src/test/org/apache/commons/pool/impl/TestGenericObjectPoolClassLoaders.java?p2=commons/proper/pool/branches/1_5_RELEASE/src/test/org/apache/commons/pool/impl/TestGenericObjectPoolClassLoaders.java&p1=commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericObjectPoolClassLoaders.java&r1=1243737&r2=1243741&rev=1243741&view=diff
==============================================================================
--- commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericObjectPoolClassLoaders.java (original)
+++ commons/proper/pool/branches/1_5_RELEASE/src/test/org/apache/commons/pool/impl/TestGenericObjectPoolClassLoaders.java Mon Feb 13 23:28:44 2012
@@ -39,7 +39,7 @@ public class TestGenericObjectPoolClassL
 			Thread.currentThread().setContextClassLoader(cl1);
 			CustomClassLoaderObjectFactory factory1 = new CustomClassLoaderObjectFactory(
 					1);
-			GenericObjectPool<URL> pool1 = new GenericObjectPool<URL>(factory1);
+			GenericObjectPool pool1 = new GenericObjectPool(factory1);
 			pool1.setMinIdle(1);
 			pool1.setTimeBetweenEvictionRunsMillis(100);
 			Thread.sleep(200);
@@ -51,7 +51,7 @@ public class TestGenericObjectPoolClassL
 			Thread.currentThread().setContextClassLoader(cl2);
 			CustomClassLoaderObjectFactory factory2 = new CustomClassLoaderObjectFactory(
 					2);
-			GenericObjectPool<URL> pool2 = new GenericObjectPool<URL>(factory2);
+			GenericObjectPool pool2 = new GenericObjectPool(factory2);
 			pool2.setMinIdle(1);
 
 			pool2.addObject();
@@ -71,21 +71,20 @@ public class TestGenericObjectPoolClassL
 	}
 
 	private class CustomClassLoaderObjectFactory extends
-			BasePoolableObjectFactory<URL> {
+			BasePoolableObjectFactory {
 		private int n;
 
 		CustomClassLoaderObjectFactory(int n) {
 			this.n = n;
 		}
 
-		@Override
-        public URL makeObject() throws Exception {
-			URL url = Thread.currentThread().getContextClassLoader()
+        public Object makeObject() throws Exception {
+            Object o = Thread.currentThread().getContextClassLoader()
 					.getResource("test" + n);
-			if (url == null) {
+			if (o == null) {
 				throw new IllegalStateException("Object should not be null");
 			}
-			return url;
+			return o;
 		}
 
 	}
@@ -98,7 +97,6 @@ public class TestGenericObjectPoolClassL
 			this.n = n;
 		}
 
-		@Override
         public URL findResource(String name) {
 			if (!name.endsWith(String.valueOf(n))) {
 				return null;