You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/09/19 17:31:44 UTC
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
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