[ 
https://issues.apache.org/jira/browse/POOL-338?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16436374#comment-16436374
 ] 

Michael C commented on POOL-338:
--------------------------------

The patch doesn't work for me, because you missed the logic that favors 
{{evictionPolicy}} in {{GenericObjectPool.setConfig()}}. Even for 
{{GenericKeyedObjectPool.setConfig()}}, I believe favoring {{evictionPolicy}} 
should simply be:
{noformat}
@@ -238,7 +238,7 @@
      *
      * @see GenericKeyedObjectPoolConfig
      */
-    public void setConfig(final GenericKeyedObjectPoolConfig conf) {
+    public void setConfig(final GenericKeyedObjectPoolConfig<T> conf) {
         setLifo(conf.getLifo());
         setMaxIdlePerKey(conf.getMaxIdlePerKey());
         setMaxTotalPerKey(conf.getMaxTotalPerKey());
@@ -256,7 +256,14 @@
                 conf.getSoftMinEvictableIdleTimeMillis());
         setTimeBetweenEvictionRunsMillis(
                 conf.getTimeBetweenEvictionRunsMillis());
-        setEvictionPolicyClassName(conf.getEvictionPolicyClassName());
+        final EvictionPolicy<T> policy = conf.getEvictionPolicy();
+        if (policy == null) {
+            // Use the class name
+            setEvictionPolicyClassName(conf.getEvictionPolicyClassName());
+        } else {
+            // Otherwise, use the class
+            setEvictionPolicy(policy);
+        }
         
setEvictorShutdownTimeoutMillis(conf.getEvictorShutdownTimeoutMillis());
     }
{noformat}
That does not break compatibility, because 
{{BaseObjectPoolConfig.evictionPolicy}} is new in 2.6.

Speaking of the 2.6 API, if anything, this bug tells us to avoid class loaders. 
Here are my assessments:

1) This static member may be _created_ by an unknown thread:

{{+ static final Class<EvictionPolicy> EVICTION_POLICY_TYPE = 
EvictionPolicy.class;}}

What is {{EVICTION_POLICY_TYPE.getClassLoader()}}? Is it from that unknown 
thread or the default class loader?

2) What is the value of {{BaseGenericObjectPool.setEvictionPolicyClass()}} when 
you already have the {{setEvictionPolicy()}} pair? It's one more potential 
problem down the road.

Thanks

> GenericObjectPool constructor throws an exception
> -------------------------------------------------
>
>                 Key: POOL-338
>                 URL: https://issues.apache.org/jira/browse/POOL-338
>             Project: Commons Pool
>          Issue Type: Bug
>    Affects Versions: 2.4.2, 2.4.3, 2.5.0
>         Environment: Java 8, Liferay DXP (an OSGi environment).
>            Reporter: Michael C
>            Priority: Major
>         Attachments: commons-pool-gg.patch
>
>
> Version 2.4.3 GenericObjectPool constructor throws this exception:
> {{java.lang.IllegalArgumentException: 
> [org.apache.commons.pool2.impl.DefaultEvictionPolicy] does not implement 
> EvictionPolicy}}
> {{    at 
> org.apache.commons.pool2.impl.BaseGenericObjectPool.setEvictionPolicyClassName(BaseGenericObjectPool.java:618)}}
> {{    at 
> org.apache.commons.pool2.impl.GenericObjectPool.setConfig(GenericObjectPool.java:318)}}
> {{    at 
> org.apache.commons.pool2.impl.GenericObjectPool.<init>(GenericObjectPool.java:115)}}
> {{    at 
> org.apache.commons.pool2.impl.GenericObjectPool.<init>(GenericObjectPool.java:88)}}
>  
> Version 2.5.0 throws the same exception. Version 2.4.2 or older's 
> setEvictionPolicyClassName method fail silently for the same reason. This 
> line in BaseGenericObjectPool evaluates to false for all versions:
> {{        if (policy instanceof EvictionPolicy<?>) {}}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to