You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@cocoon.apache.org by sy...@apache.org on 2005/09/14 11:38:56 UTC
svn commit: r280807 - in
/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon: Cocoon.java
components/flow/javascript/LocationTrackingDebugger.java
util/location/LocationUtils.java
Author: sylvain
Date: Wed Sep 14 02:38:48 2005
New Revision: 280807
URL: http://svn.apache.org/viewcvs?rev=280807&view=rev
Log:
Fix a potential memory leak if classes in other classloaders register location finders
Modified:
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/Cocoon.java
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/flow/javascript/LocationTrackingDebugger.java
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/util/location/LocationUtils.java
Modified: cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/Cocoon.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/Cocoon.java?rev=280807&r1=280806&r2=280807&view=diff
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/Cocoon.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/Cocoon.java Wed Sep 14 02:38:48 2005
@@ -92,51 +92,52 @@
InstrumentManageable {
// 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/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/flow/javascript/LocationTrackingDebugger.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/flow/javascript/LocationTrackingDebugger.java?rev=280807&r1=280806&r2=280807&view=diff
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/flow/javascript/LocationTrackingDebugger.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/flow/javascript/LocationTrackingDebugger.java Wed Sep 14 02:38:48 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/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/util/location/LocationUtils.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/util/location/LocationUtils.java?rev=280807&r1=280806&r2=280807&view=diff
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/util/location/LocationUtils.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/util/location/LocationUtils.java Wed Sep 14 02:38:48 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;
}
}