tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1172664 - in /tomcat/trunk: java/org/apache/catalina/core/DefaultInstanceManager.java test/org/apache/catalina/core/TestDefaultInstanceManager.java test/webapp-3.0/annotations.jsp
Date Mon, 19 Sep 2011 15:31:44 GMT
Author: markt
Date: Mon Sep 19 15:31:43 2011
New Revision: 1172664

URL: http://svn.apache.org/viewvc?rev=1172664&view=rev
Log:
Fix resource leak in annotations cache that prevented unloading of resources that used annotations

Added:
    tomcat/trunk/test/org/apache/catalina/core/TestDefaultInstanceManager.java   (with props)
    tomcat/trunk/test/webapp-3.0/annotations.jsp   (with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java

Modified: tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java?rev=1172664&r1=1172663&r2=1172664&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java Mon Sep 19 15:31:43
2011
@@ -21,6 +21,7 @@ package org.apache.catalina.core;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.ref.WeakReference;
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
@@ -35,7 +36,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.WeakHashMap;
 
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
@@ -70,8 +71,8 @@ public class DefaultInstanceManager impl
     private Properties restrictedFilters = new Properties();
     private Properties restrictedListeners = new Properties();
     private Properties restrictedServlets = new Properties();
-    private Map<Class<?>,List<AnnotationCacheEntry>> annotationCache =
-        new ConcurrentHashMap<Class<?>, List<AnnotationCacheEntry>>();
+    private final Map<Class<?>,WeakReference<List<AnnotationCacheEntry>>>
annotationCache =
+        new WeakHashMap<Class<?>, WeakReference<List<AnnotationCacheEntry>>>();
 
     public DefaultInstanceManager(Context context, Map<String, Map<String, String>>
injectionMap, org.apache.catalina.Context catalinaContext, ClassLoader containerClassLoader)
{
         classLoader = catalinaContext.getLoader().getClassLoader();
@@ -178,7 +179,10 @@ public class DefaultInstanceManager impl
 
         // At the end the postconstruct annotated
         // method is invoked
-        List<AnnotationCacheEntry> annotations = annotationCache.get(clazz);
+        List<AnnotationCacheEntry> annotations;
+        synchronized (annotationCache) {
+            annotations = annotationCache.get(clazz).get();
+        }
         for (AnnotationCacheEntry entry : annotations) {
             if (entry.getType() == AnnotationCacheEntryType.POST_CONSTRUCT) {
                 Method postConstruct = (Method) entry.getAccessibleObject();
@@ -209,7 +213,14 @@ public class DefaultInstanceManager impl
 
         // At the end the postconstruct annotated
         // method is invoked
-        List<AnnotationCacheEntry> annotations = annotationCache.get(clazz);
+        List<AnnotationCacheEntry> annotations = null;
+        synchronized (annotationCache) {
+            WeakReference<List<AnnotationCacheEntry>> ref =
+                annotationCache.get(clazz);
+            if (ref != null) {
+                annotations = ref.get();
+            }
+        }
         if (annotations == null) {
             // instance not created through the instance manager
             return;
@@ -243,7 +254,14 @@ public class DefaultInstanceManager impl
             InvocationTargetException, NamingException {
 
         while (clazz != null) {
-            List<AnnotationCacheEntry> annotations = annotationCache.get(clazz);
+            List<AnnotationCacheEntry> annotations = null;
+            synchronized (annotationCache) {
+                WeakReference<List<AnnotationCacheEntry>> ref =
+                    annotationCache.get(clazz);
+                if (ref != null) {
+                    annotations = ref.get();
+                }
+            }
             if (annotations == null) {
                 annotations = new ArrayList<AnnotationCacheEntry>();
                 
@@ -396,7 +414,11 @@ public class DefaultInstanceManager impl
                     // Use common empty list to save memory 
                     annotations = Collections.emptyList();
                 }
-                annotationCache.put(clazz, annotations);
+                synchronized (annotationCache) {
+                    annotationCache.put(clazz,
+                            new WeakReference<List<AnnotationCacheEntry>>(
+                                    annotations));
+                }
             } else {
                 // If the annotations for this class have been cached, the
                 // annotations for all the super classes will have been cachced
@@ -429,7 +451,10 @@ public class DefaultInstanceManager impl
         Class<?> clazz = instance.getClass();
         
         while (clazz != null) {
-            List<AnnotationCacheEntry> annotations = annotationCache.get(clazz);
+            List<AnnotationCacheEntry> annotations;
+            synchronized (annotationCache) {
+                annotations = annotationCache.get(clazz).get();
+            }
             for (AnnotationCacheEntry entry : annotations) {
                 if (entry.getType() == AnnotationCacheEntryType.FIELD) {
                     if (entry.getAccessibleObject() instanceof Method) {
@@ -448,6 +473,16 @@ public class DefaultInstanceManager impl
     }
 
 
+    /**
+     * Makes cache size available to unit tests.
+     */
+    protected int getAnnotationCacheSize() {
+        synchronized (annotationCache) {
+            return annotationCache.size();
+        }
+    }
+
+
     protected Class<?> loadClassMaybePrivileged(final String className, final ClassLoader
classLoader) throws ClassNotFoundException {
         Class<?> clazz;
         if (SecurityUtil.isPackageProtectionEnabled()) {

Added: tomcat/trunk/test/org/apache/catalina/core/TestDefaultInstanceManager.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestDefaultInstanceManager.java?rev=1172664&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/core/TestDefaultInstanceManager.java (added)
+++ tomcat/trunk/test/org/apache/catalina/core/TestDefaultInstanceManager.java Mon Sep 19
15:31:43 2011
@@ -0,0 +1,86 @@
+/*
+ * 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.catalina.core;
+
+import java.io.File;
+
+import static org.junit.Assert.assertEquals;
+import org.junit.Test;
+
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+
+
+public class TestDefaultInstanceManager extends TomcatBaseTest {
+
+    @Test
+    public void testClassUnloading() throws Exception {
+        
+        DefaultInstanceManager instanceManager = doClassUnloadingPrep();
+
+        // Request a JSP page (that doesn't load any tag libraries etc.)
+        // This page does use @PostConstruct to ensure that the cache does not
+        // retain strong references
+        getUrl("http://localhost:" + getPort() + "/test/annotations.jsp");
+        // Request a second JSP (again, no tag libraries etc.)
+        getUrl("http://localhost:" + getPort() + "/test/bug36923.jsp");
+        
+        // Check the number of classes in the cache
+        int count = instanceManager.getAnnotationCacheSize();
+
+        // Request a third JSP (again, no tag libraries etc.)
+        getUrl("http://localhost:" + getPort() + "/test/bug51544.jsp");
+        
+        // Force a GC to clear out unloaded class (first JSP)
+        System.gc();
+
+        // Spin a while until GC happens or we wait too long
+        int loop = 0;
+        while (loop < 10) {
+            if (instanceManager.getAnnotationCacheSize() == count) {
+                break;
+            }
+            Thread.sleep(100);
+            loop++;
+        }
+
+        // First JSP should be unloaded and replaced by third (second left
+        // alone) so no change in overall count
+        assertEquals(count, instanceManager.getAnnotationCacheSize());
+    }
+
+    private DefaultInstanceManager doClassUnloadingPrep() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+        
+        // Create the context (don't use addWebapp as we want to modify the
+        // JSP Servlet settings).
+        File appDir = new File("test/webapp-3.0");
+        StandardContext ctxt = (StandardContext) tomcat.addContext(
+                null, "/test", appDir.getAbsolutePath());
+        
+        // Configure the defaults and then tweak the JSP servlet settings
+        // Note: Min value for maxLoadedJsps is 2
+        Tomcat.initWebappDefaults(ctxt);
+        Wrapper w = (Wrapper) ctxt.findChild("jsp");
+        w.addInitParameter("maxLoadedJsps", "2");
+        
+        tomcat.start();
+        
+        return (DefaultInstanceManager) ctxt.getInstanceManager();
+    }
+}

Propchange: tomcat/trunk/test/org/apache/catalina/core/TestDefaultInstanceManager.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: tomcat/trunk/test/webapp-3.0/annotations.jsp
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/webapp-3.0/annotations.jsp?rev=1172664&view=auto
==============================================================================
--- tomcat/trunk/test/webapp-3.0/annotations.jsp (added)
+++ tomcat/trunk/test/webapp-3.0/annotations.jsp Mon Sep 19 15:31:43 2011
@@ -0,0 +1,28 @@
+<%--
+ 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.
+--%>
+<%@page import="javax.annotation.PostConstruct"%>
+<html>
+  <head><title>Annotations test case</title></head>
+  <body>
+    <p>Hello world</p>
+  </body>
+</html>
+<%!
+  @PostConstruct
+  public void doNothing() {
+  }
+%>
\ No newline at end of file

Propchange: tomcat/trunk/test/webapp-3.0/annotations.jsp
------------------------------------------------------------------------------
    svn:eol-style = native



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message