You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by ff...@apache.org on 2019/01/15 03:14:21 UTC

[cxf] branch master updated: [CXF-7934]use ClassValue for ProxyClassLoader cache

This is an automated email from the ASF dual-hosted git repository.

ffang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cxf.git


The following commit(s) were added to refs/heads/master by this push:
     new 815aa8a  [CXF-7934]use ClassValue for ProxyClassLoader cache
815aa8a is described below

commit 815aa8ae002bb6d9730b272cc946535d1846ca1d
Author: Freeman Fang <fr...@gmail.com>
AuthorDate: Mon Dec 24 16:55:50 2018 +0800

    [CXF-7934]use ClassValue for ProxyClassLoader cache
    
    (cherry picked from commit 3404bebf755e5e0b656a1820b42ed90ec37a91e9)
---
 .../cxf/common/util/ProxyClassLoaderCache.java     | 117 +++++++++++++++++++++
 .../org/apache/cxf/common/util/ProxyHelper.java    |  80 ++++----------
 .../cxf/common/util/ProxyClassLoaderCacheTest.java |  97 +++++++++++++++++
 .../org/apache/cxf/jaxrs/utils/InjectionUtils.java |  38 +++----
 .../cxf/frontend/ClientProxyFactoryBean.java       |   2 +-
 5 files changed, 252 insertions(+), 82 deletions(-)

diff --git a/core/src/main/java/org/apache/cxf/common/util/ProxyClassLoaderCache.java b/core/src/main/java/org/apache/cxf/common/util/ProxyClassLoaderCache.java
new file mode 100644
index 0000000..dd69a13
--- /dev/null
+++ b/core/src/main/java/org/apache/cxf/common/util/ProxyClassLoaderCache.java
@@ -0,0 +1,117 @@
+/**
+ * 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.cxf.common.util;
+
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import org.apache.cxf.common.logging.LogUtils;
+
+public class ProxyClassLoaderCache {
+    
+    private static final Logger LOG = LogUtils.getL7dLogger(ProxyClassLoaderCache.class);
+    private static final ThreadLocal<ClassLoader> PARENT_CLASSLOADER = new ThreadLocal<>();
+    private static final ThreadLocal<Class<?>[]> PROXY_INTERFACES = new ThreadLocal<>();
+    
+
+    
+    private final ClassValue<ClassLoader> backend = new ClassValue<ClassLoader>() {
+        @Override
+        protected ClassLoader computeValue(Class<?> proxyInterface) {
+            LOG.log(Level.FINE, "can't find ProxyClassLoader from ClassValue Cache, "
+                + "will create a new one");
+            LOG.log(Level.FINE, "interface for new created ProxyClassLoader is "
+                + proxyInterface.getName());
+            LOG.log(Level.FINE, "interface's classloader for new created ProxyClassLoader is "
+                + proxyInterface.getClassLoader());
+            return createProxyClassLoader(proxyInterface);
+        }
+
+    };
+
+    private ClassLoader createProxyClassLoader(Class<?> proxyInterface) {
+        final SecurityManager sm = System.getSecurityManager();
+        ProxyClassLoader ret = null;
+        if (sm == null) {
+            ret = new ProxyClassLoader(PARENT_CLASSLOADER.get(), PROXY_INTERFACES.get());
+        } else {
+            ret = AccessController.doPrivileged(new PrivilegedAction<ProxyClassLoader>() {
+                @Override
+                public ProxyClassLoader run() {
+                    return new ProxyClassLoader(PARENT_CLASSLOADER.get(), PROXY_INTERFACES.get());
+                }
+            });
+        }
+        for (Class<?> currentInterface : PROXY_INTERFACES.get()) {
+            ret.addLoader(getClassLoader(currentInterface));
+            LOG.log(Level.FINE, "interface for new created ProxyClassLoader is "
+                + currentInterface.getName());
+            LOG.log(Level.FINE, "interface's classloader for new created ProxyClassLoader is "
+                + currentInterface.getClassLoader());
+        }
+        return ret;
+    }
+
+      
+    public ClassLoader getProxyClassLoader(ClassLoader parent, Class<?>[] proxyInterfaces) {
+        try {
+            PARENT_CLASSLOADER.set(parent);
+            PROXY_INTERFACES.set(proxyInterfaces);
+            for (Class<?> currentInterface : proxyInterfaces) {
+                String ifName = currentInterface.getName();
+                LOG.log(Level.FINE, "the interface we are checking is " + currentInterface.getName());
+                LOG.log(Level.FINE, "the interface' classloader we are checking is " 
+                    + currentInterface.getClassLoader());
+                if (!ifName.startsWith("org.apache.cxf") && !ifName.startsWith("java")) {
+                    // cache and retrieve customer interface
+                    LOG.log(Level.FINE, "the customer interface is " + currentInterface.getName()
+                                        + ". Will try to fetch it from Cache");
+                    return backend.get(currentInterface);
+                }
+            }
+            LOG.log(Level.FINE, "Non of interfaces are customer interface, "
+                + "retrive the last interface as key:" 
+                + proxyInterfaces[proxyInterfaces.length - 1].getName());
+            //the last interface is the variable type
+            return backend.get(proxyInterfaces[proxyInterfaces.length - 1]);
+        } finally {
+            PARENT_CLASSLOADER.remove();
+            PROXY_INTERFACES.remove();
+        }
+    }
+    
+    public void removeStaleProxyClassLoader(Class<?> proxyInterface) {
+        backend.remove(proxyInterface);
+    }
+    
+    private static ClassLoader getClassLoader(final Class<?> clazz) {
+        final SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            return AccessController.doPrivileged(new PrivilegedAction<ClassLoader>() {
+                public ClassLoader run() {
+                    return clazz.getClassLoader();
+                }
+            });
+        }
+        return clazz.getClassLoader();
+    }
+}
diff --git a/core/src/main/java/org/apache/cxf/common/util/ProxyHelper.java b/core/src/main/java/org/apache/cxf/common/util/ProxyHelper.java
index ad83ad9..04e4ee2 100644
--- a/core/src/main/java/org/apache/cxf/common/util/ProxyHelper.java
+++ b/core/src/main/java/org/apache/cxf/common/util/ProxyHelper.java
@@ -22,11 +22,6 @@ package org.apache.cxf.common.util;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -49,11 +44,9 @@ public class ProxyHelper {
    
     private static final Logger LOG = LogUtils.getL7dLogger(ProxyHelper.class);
     
-    protected Map<String, ClassLoader> proxyClassLoaderCache = 
-        Collections.synchronizedMap(new HashMap<String, ClassLoader>());
-    protected int cacheSize =
-        Integer.parseInt(System.getProperty("org.apache.cxf.proxy.classloader.size", "3000"));
-    
+    protected ProxyClassLoaderCache proxyClassLoaderCache = 
+        new ProxyClassLoaderCache();
+      
     
     protected ProxyHelper() {
     }
@@ -77,46 +70,28 @@ public class ProxyHelper {
             return loader;
         }
         String sortedNameFromInterfaceArray = getSortedNameFromInterfaceArray(interfaces);
-        ClassLoader cachedLoader = proxyClassLoaderCache.get(sortedNameFromInterfaceArray);
-        if (cachedLoader != null) {
-            if (canSeeAllInterfaces(cachedLoader, interfaces)) {
-                //found cached loader
-                LOG.log(Level.FINE, "find required loader from ProxyClassLoader cache with key" 
-                        + sortedNameFromInterfaceArray);
-                return cachedLoader;
-            } else {
-                //found cached loader somehow can't see all interfaces
-                LOG.log(Level.FINE, "find a loader from ProxyClassLoader cache with key " 
-                        + sortedNameFromInterfaceArray
-                        + " but can't see all interfaces");
-            }
-        }
-        ProxyClassLoader combined;
-        LOG.log(Level.FINE, "can't find required ProxyClassLoader from cache, create a new one with parent " + loader);
-        final SecurityManager sm = System.getSecurityManager();
-        if (sm == null) {
-            combined = new ProxyClassLoader(loader, interfaces);
+        ClassLoader cachedLoader = proxyClassLoaderCache.getProxyClassLoader(loader, interfaces);
+        if (canSeeAllInterfaces(cachedLoader, interfaces)) {
+            LOG.log(Level.FINE, "find required loader from ProxyClassLoader cache with key" 
+                 + sortedNameFromInterfaceArray);
+            return cachedLoader;
         } else {
-            combined = AccessController.doPrivileged(new PrivilegedAction<ProxyClassLoader>() {
-                @Override
-                public ProxyClassLoader run() {
-                    return new ProxyClassLoader(loader, interfaces);
+            LOG.log(Level.FINE, "find a loader from ProxyClassLoader cache with interfaces " 
+                + sortedNameFromInterfaceArray
+                + " but can't see all interfaces");
+            for (Class<?> currentInterface : interfaces) {
+                String ifName = currentInterface.getName();
+                
+                if (!ifName.startsWith("org.apache.cxf") && !ifName.startsWith("java")) {
+                    // remove the stale ProxyClassLoader and recreate one
+                    proxyClassLoaderCache.removeStaleProxyClassLoader(currentInterface);
+                    cachedLoader = proxyClassLoaderCache.getProxyClassLoader(loader, interfaces);
+                    
                 }
-            });
-        }
-        for (Class<?> currentInterface : interfaces) {
-            combined.addLoader(getClassLoader(currentInterface));
-            LOG.log(Level.FINE, "interface for new created ProxyClassLoader is "
-                + currentInterface.getName());
-            LOG.log(Level.FINE, "interface's classloader for new created ProxyClassLoader is "
-                + currentInterface.getClassLoader());
-        }
-        if (proxyClassLoaderCache.size() >= cacheSize) {
-            LOG.log(Level.FINE, "proxyClassLoaderCache is full, need clear it");
-            proxyClassLoaderCache.clear();
+            }
         }
-        proxyClassLoaderCache.put(sortedNameFromInterfaceArray, combined);
-        return combined;
+               
+        return cachedLoader;
     }
     
     private String getSortedNameFromInterfaceArray(Class<?>[] interfaces) {
@@ -127,17 +102,6 @@ public class ProxyHelper {
         return arraySet.toString();
     }
 
-    private static ClassLoader getClassLoader(final Class<?> clazz) {
-        final SecurityManager sm = System.getSecurityManager();
-        if (sm != null) {
-            return AccessController.doPrivileged(new PrivilegedAction<ClassLoader>() {
-                public ClassLoader run() {
-                    return clazz.getClassLoader();
-                }
-            });
-        }
-        return clazz.getClassLoader();
-    }
 
     private boolean canSeeAllInterfaces(ClassLoader loader, Class<?>[] interfaces) {
         for (Class<?> currentInterface : interfaces) {
diff --git a/core/src/test/java/org/apache/cxf/common/util/ProxyClassLoaderCacheTest.java b/core/src/test/java/org/apache/cxf/common/util/ProxyClassLoaderCacheTest.java
new file mode 100644
index 0000000..dd7d974
--- /dev/null
+++ b/core/src/test/java/org/apache/cxf/common/util/ProxyClassLoaderCacheTest.java
@@ -0,0 +1,97 @@
+/**
+ * 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.cxf.common.util;
+
+
+
+import java.io.Closeable;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+
+import org.apache.cxf.endpoint.Client;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ProxyClassLoaderCacheTest extends Assert {
+    
+    private ProxyClassLoaderCache cache;
+    
+    @Test
+    public void testClassLoaderIdentical() throws Exception {
+        cache = new ProxyClassLoaderCache();
+        ClassLoader cl1 = cache.getProxyClassLoader(
+            this.getClass().getClassLoader(), 
+            new Class<?>[]{Closeable.class, Client.class,  HelloWorld.class});
+        ClassLoader cl2 = cache.getProxyClassLoader(
+            this.getClass().getClassLoader(), 
+            new Class<?>[]{Closeable.class, Client.class,  HelloWorld.class});
+        assertTrue(cl1 == cl2);
+    }
+    
+    @Test
+    public void testClassLoaderIdenticalWithMultipleThreads() throws Exception {
+        cache = new ProxyClassLoaderCache();
+        Set<ClassLoader> clSet = Collections.synchronizedSet(new HashSet<>());
+        CountDownLatch countDownLatch = new CountDownLatch(50);
+        for (int i = 0; i < 50; i++) {
+            new Thread(new HelloWorker(clSet, countDownLatch)).start();
+        }
+        countDownLatch.await(); 
+        assertTrue(clSet.size() == 1);
+    }
+            
+    interface HelloWorld {
+        void sayHello();
+    }
+    
+    class HelloWorker implements Runnable {
+
+        private Set<ClassLoader> classLoaderSet;
+        
+        private CountDownLatch doneSignal;
+        HelloWorker(Set<ClassLoader> classLoaderSet,
+                           CountDownLatch doneSignal) {
+            this.classLoaderSet = classLoaderSet;
+            this.doneSignal = doneSignal;
+        }
+
+        public void run() {
+            
+
+            try {
+                this.classLoaderSet.add(cache.getProxyClassLoader(
+                                        this.getClass().getClassLoader(), 
+                                        new Class<?>[]{Closeable.class, 
+                                        Client.class,  
+                                        HelloWorld.class}));
+                doneSignal.countDown();
+           
+            } catch (RuntimeException ex) {
+                ex.printStackTrace();
+                
+            }
+
+        }
+
+    }
+}
diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/InjectionUtils.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/InjectionUtils.java
index e2c80e1..fd83ad6 100644
--- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/InjectionUtils.java
+++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/InjectionUtils.java
@@ -72,7 +72,7 @@ import org.apache.cxf.common.i18n.BundleUtils;
 import org.apache.cxf.common.logging.LogUtils;
 import org.apache.cxf.common.util.ClassHelper;
 import org.apache.cxf.common.util.PrimitiveUtils;
-import org.apache.cxf.common.util.ProxyClassLoader;
+import org.apache.cxf.common.util.ProxyClassLoaderCache;
 import org.apache.cxf.common.util.ReflectionUtil;
 import org.apache.cxf.common.util.StringUtils;
 import org.apache.cxf.helpers.CastUtils;
@@ -138,11 +138,8 @@ public final class InjectionUtils {
 
     private static final String IGNORE_MATRIX_PARAMETERS = "ignore.matrix.parameters";
     
-    private static Map<String, ProxyClassLoader> proxyClassLoaderCache = 
-        Collections.synchronizedMap(new HashMap<String, ProxyClassLoader>());
-    
-    private static int cacheSize =
-        Integer.parseInt(System.getProperty("org.apache.cxf.proxy.classloader.size", "3000"));
+    private static ProxyClassLoaderCache proxyClassLoaderCache = 
+        new ProxyClassLoaderCache();
     
     private InjectionUtils() {
 
@@ -1082,24 +1079,19 @@ public final class InjectionUtils {
             proxy = createThreadLocalServletApiContext(type.getName());
         }
         if (proxy == null) {
-            ProxyClassLoader loader
-                = proxyClassLoaderCache.get(type.getName() + type.getClassLoader()); 
-            if (loader == null
-                || !canSeeAllClasses(loader, new Class<?>[]{Proxy.class, type, ThreadLocalProxy.class})) {
-                // to avoid creating too much ProxyClassLoader to save Metaspace usage
-                LOG.log(Level.FINE, "can't find required ProxyClassLoader for type " + type.getName());
+            ClassLoader loader
+                = proxyClassLoaderCache.getProxyClassLoader(Proxy.class.getClassLoader(), 
+                                                            new Class<?>[]{Proxy.class, ThreadLocalProxy.class, type}); 
+            if (!canSeeAllClasses(loader, new Class<?>[]{Proxy.class, ThreadLocalProxy.class, type})) {
+                LOG.log(Level.FINE, "find a loader from ProxyClassLoader cache," 
+                    + " but can't see all interfaces");
+                
                 LOG.log(Level.FINE, "create a new one with parent  " + Proxy.class.getClassLoader());
-                loader = new ProxyClassLoader(Proxy.class.getClassLoader());
-                loader.addLoader(type.getClassLoader());
-                LOG.log(Level.FINE, "type classloader is  " + type.getClassLoader());
-                loader.addLoader(ThreadLocalProxy.class.getClassLoader());
-                LOG.log(Level.FINE, "ThreadLocalProxy classloader is  " 
-                    + ThreadLocalProxy.class.getClassLoader().getClass().getName());
-                if (proxyClassLoaderCache.size() >= cacheSize) {
-                    LOG.log(Level.FINE, "proxyClassLoaderCache is full, need clear it");
-                    proxyClassLoaderCache.clear();
-                }
-                proxyClassLoaderCache.put(type.getName() + type.getClassLoader(), loader); 
+                proxyClassLoaderCache.removeStaleProxyClassLoader(type);
+                proxyClassLoaderCache.getProxyClassLoader(Proxy.class.getClassLoader(), 
+                                                          new Class<?>[]{Proxy.class, ThreadLocalProxy.class, type}); 
+                
+                
             } 
             return (ThreadLocalProxy<T>)Proxy.newProxyInstance(loader,
                                    new Class[] {type, ThreadLocalProxy.class },
diff --git a/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ClientProxyFactoryBean.java b/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ClientProxyFactoryBean.java
index 6a2c69a..d7b2977 100644
--- a/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ClientProxyFactoryBean.java
+++ b/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ClientProxyFactoryBean.java
@@ -201,7 +201,7 @@ public class ClientProxyFactoryBean extends AbstractBasicInterceptorProvider {
 
     protected Class<?>[] getImplementingClasses() {
         Class<?> cls = clientFactoryBean.getServiceClass();
-        return new Class[] {cls, Closeable.class, Client.class};
+        return new Class[] {Closeable.class, Client.class, cls};
     }
 
     protected ClientProxy clientClientProxy(Client c) {