cocoon-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sylv...@apache.org
Subject svn commit: r280811 - in /cocoon/trunk/src/java/org/apache/cocoon: Cocoon.java components/flow/javascript/LocationTrackingDebugger.java util/location/LocationUtils.java
Date Wed, 14 Sep 2005 09:53:37 GMT
Author: sylvain
Date: Wed Sep 14 02:53:32 2005
New Revision: 280811

URL: http://svn.apache.org/viewcvs?rev=280811&view=rev
Log:
Fix a potential memory leak if classes in other classloaders register location finders

Modified:
    cocoon/trunk/src/java/org/apache/cocoon/Cocoon.java
    cocoon/trunk/src/java/org/apache/cocoon/components/flow/javascript/LocationTrackingDebugger.java
    cocoon/trunk/src/java/org/apache/cocoon/util/location/LocationUtils.java

Modified: cocoon/trunk/src/java/org/apache/cocoon/Cocoon.java
URL: http://svn.apache.org/viewcvs/cocoon/trunk/src/java/org/apache/cocoon/Cocoon.java?rev=280811&r1=280810&r2=280811&view=diff
==============================================================================
--- cocoon/trunk/src/java/org/apache/cocoon/Cocoon.java (original)
+++ cocoon/trunk/src/java/org/apache/cocoon/Cocoon.java Wed Sep 14 02:53:32 2005
@@ -82,51 +82,52 @@
                    Serviceable {
 
     // Register the location finder for Avalon configuration objects and exceptions
-    static {
-        LocationUtils.addFinder(new LocationUtils.LocationFinder() {
-
-            public Location getLocation(Object obj, String description) {
-                if (obj instanceof Configuration) {
-                    Configuration config = (Configuration)obj;
-                    String locString = config.getLocation();
-                    Location result = LocationUtils.parse(locString);
-                    if (LocationUtils.isKnown(result)) {
-                        // Add description
-                        StringBuffer desc = new StringBuffer().append('<');
-                        // Unfortunately Configuration.getPrefix() is not public
-                        try {
-                            if (config.getNamespace().startsWith("http://apache.org/cocoon/sitemap/"))
{
-                                desc.append("map:");
-                            }
-                        } catch (ConfigurationException e) {
-                            // no namespace: ignore
+    // and keep a strong reference to it.
+    private static final LocationUtils.LocationFinder confLocFinder = new LocationUtils.LocationFinder()
{
+        public Location getLocation(Object obj, String description) {
+            if (obj instanceof Configuration) {
+                Configuration config = (Configuration)obj;
+                String locString = config.getLocation();
+                Location result = LocationUtils.parse(locString);
+                if (LocationUtils.isKnown(result)) {
+                    // Add description
+                    StringBuffer desc = new StringBuffer().append('<');
+                    // Unfortunately Configuration.getPrefix() is not public
+                    try {
+                        if (config.getNamespace().startsWith("http://apache.org/cocoon/sitemap/"))
{
+                            desc.append("map:");
                         }
-                        desc.append(config.getName()).append('>');
-                        return new LocationImpl(desc.toString(), result);
-                    } else {
-                        return result;
+                    } catch (ConfigurationException e) {
+                        // no namespace: ignore
                     }
+                    desc.append(config.getName()).append('>');
+                    return new LocationImpl(desc.toString(), result);
+                } else {
+                    return result;
                 }
+            }
+            
+            if (obj instanceof Exception) {
+                // Many exceptions in Cocoon have a message like "blah blah at file://foo/bar.xml:12:1"
+                String msg = ((Exception)obj).getMessage();
+                if (msg == null) return null;
                 
-                if (obj instanceof Exception) {
-                    // Many exceptions in Cocoon have a message like "blah blah at file://foo/bar.xml:12:1"
-                    String msg = ((Exception)obj).getMessage();
-                    if (msg == null) return null;
-                    
-                    int pos = msg.lastIndexOf(" at ");
-                    if (pos != -1) {
-                        return LocationUtils.parse(msg.substring(pos + 4));
-                    } else {
-                        // Will try other finders
-                        return null;
-                    }
+                int pos = msg.lastIndexOf(" at ");
+                if (pos != -1) {
+                    return LocationUtils.parse(msg.substring(pos + 4));
+                } else {
+                    // Will try other finders
+                    return null;
                 }
-                
-                // Try next finders.
-                return null;
             }
-        });
-        
+            
+            // Try next finders.
+            return null;
+        }
+    };
+    
+    static {
+        LocationUtils.addFinder(confLocFinder);
     }
     
     static Cocoon instance;

Modified: cocoon/trunk/src/java/org/apache/cocoon/components/flow/javascript/LocationTrackingDebugger.java
URL: http://svn.apache.org/viewcvs/cocoon/trunk/src/java/org/apache/cocoon/components/flow/javascript/LocationTrackingDebugger.java?rev=280811&r1=280810&r2=280811&view=diff
==============================================================================
--- cocoon/trunk/src/java/org/apache/cocoon/components/flow/javascript/LocationTrackingDebugger.java
(original)
+++ cocoon/trunk/src/java/org/apache/cocoon/components/flow/javascript/LocationTrackingDebugger.java
Wed Sep 14 02:53:32 2005
@@ -44,32 +44,36 @@
  */
 public class LocationTrackingDebugger implements Debugger {
     
+    // Strong reference to the location finder
+    private static final LocationUtils.LocationFinder rhinoLocFinder = new LocationUtils.LocationFinder()
{
+
+        public Location getLocation(Object obj, String description) {
+            if (obj instanceof EcmaError) {
+                EcmaError ex = (EcmaError)obj;
+                if (ex.getSourceName() != null) {
+                    return new LocationImpl(ex.getName(), ex.getSourceName(), ex.getLineNumber(),
ex.getColumnNumber());
+                } else {
+                    return Location.UNKNOWN;
+                }
+    
+            } else if (obj instanceof JavaScriptException) {
+                JavaScriptException ex = (JavaScriptException)obj;
+                if (ex.sourceName() != null) {
+                    return new LocationImpl(description, ex.sourceName(), ex.lineNumber(),
-1);
+                } else {
+                    return Location.UNKNOWN;
+                }
+            }
+            
+            return null;
+        } 
+    };
+
+    
     static {
         // Register what's needed to analyze exceptions produced by Rhino
         ExceptionUtils.addCauseMethodName("getWrappedException");
-        LocationUtils.addFinder(new LocationUtils.LocationFinder() {
-
-            public Location getLocation(Object obj, String description) {
-                if (obj instanceof EcmaError) {
-                    EcmaError ex = (EcmaError)obj;
-                    if (ex.getSourceName() != null) {
-                        return new LocationImpl(ex.getName(), ex.getSourceName(), ex.getLineNumber(),
ex.getColumnNumber());
-                    } else {
-                        return Location.UNKNOWN;
-                    }
-        
-                } else if (obj instanceof JavaScriptException) {
-                    JavaScriptException ex = (JavaScriptException)obj;
-                    if (ex.sourceName() != null) {
-                        return new LocationImpl(description, ex.sourceName(), ex.lineNumber(),
-1);
-                    } else {
-                        return Location.UNKNOWN;
-                    }
-                }
-                
-                return null;
-            } 
-        });
+        LocationUtils.addFinder(rhinoLocFinder);
     }
     
     private List locations;

Modified: cocoon/trunk/src/java/org/apache/cocoon/util/location/LocationUtils.java
URL: http://svn.apache.org/viewcvs/cocoon/trunk/src/java/org/apache/cocoon/util/location/LocationUtils.java?rev=280811&r1=280810&r2=280811&view=diff
==============================================================================
--- cocoon/trunk/src/java/org/apache/cocoon/util/location/LocationUtils.java (original)
+++ cocoon/trunk/src/java/org/apache/cocoon/util/location/LocationUtils.java Wed Sep 14 02:53:32
2005
@@ -15,6 +15,10 @@
  */
 package org.apache.cocoon.util.location;
 
+import java.lang.ref.WeakReference;
+import java.util.ArrayList;
+import java.util.List;
+
 import javax.xml.transform.SourceLocator;
 import javax.xml.transform.TransformerException;
 
@@ -29,7 +33,7 @@
  */
 public class LocationUtils {
     
-    private static LocationFinder[] finders = new LocationFinder[] {};
+    private static List finders = new ArrayList();
     
     /**
      * An finder or object locations
@@ -121,6 +125,24 @@
     /**
      * Add a {@link LocationFinder} to the list of finders that will be queried for an object's
      * location by {@link #getLocation(Object, String)}.
+     * <p>
+     * <b>Important:</b> LocationUtils internally stores a weak reference to
the finder. This
+     * avoids creating strong links between the classloader holding this class and the finder's
+     * classloader, which can cause some weird memory leaks if the finder's classloader is
to
+     * be reloaded. Therefore, you <em>have</em> to keep a strong reference to
the finder in the
+     * calling code, e.g.:
+     * <pre>
+     *   private static LocationUtils.LocationFinder myFinder =
+     *       new LocationUtils.LocationFinder() {
+     *           public Location getLocation(Object obj, String desc) {
+     *               ...
+     *           }
+     *       };
+     *
+     *   static {
+     *       LocationUtils.addFinder(myFinder);
+     *   }
+     * </pre>
      * 
      * @param finder the location finder to add
      */
@@ -129,11 +151,11 @@
             return;
         }
 
-        synchronized(LocationUtils.class) {
-            int length = finders.length;
-            LocationFinder[] newFinders = new LocationFinder[length + 1];
-            System.arraycopy(finders, 0, newFinders, 0, length);
-            newFinders[length] = finder;
+        synchronized(LocationFinder.class) {
+            // Update a clone of the current finder list to avoid breaking
+            // any iteration occuring in another thread.
+            List newFinders = new ArrayList(finders);
+            newFinders.add(new WeakReference(finder));
             finders = newFinders;
         }
     }
@@ -192,15 +214,27 @@
             }
         }
 
-        Location result;
-        LocationFinder[] currentFinders = finders;
-        for (int i = 0; i < currentFinders.length; i++) {
-            result = currentFinders[i].getLocation(obj, description);
+        List currentFinders = finders; // Keep the current list
+        int size = currentFinders.size();
+        for (int i = 0; i < size; i++) {
+            WeakReference ref = (WeakReference)currentFinders.get(i);
+            LocationFinder finder = (LocationFinder)ref.get();
+            if (finder == null) {
+                // This finder was garbage collected: update finders
+                synchronized(LocationFinder.class) {
+                    // Update a clone of the current list to avoid breaking current iterations
+                    List newFinders = new ArrayList(finders);
+                    newFinders.remove(ref);
+                    finders = newFinders;
+                }
+            }
+            
+            Location result = finder.getLocation(obj, description);
             if (result != null) {
                 return result;
             }
         }
-        
+
         return Location.UNKNOWN;
     }
 }



Mime
View raw message