Author: markus Date: Fri Jan 24 13:12:00 2014 New Revision: 1560985 URL: http://svn.apache.org/r1560985 Log: NUTCH-356 Plugin repository cache can lead to memory leak
Modified: nutch/trunk/CHANGES.txt nutch/trunk/src/java/org/apache/nutch/plugin/Extension.java nutch/trunk/src/java/org/apache/nutch/plugin/PluginClassLoader.java nutch/trunk/src/java/org/apache/nutch/plugin/PluginRepository.java Modified: nutch/trunk/CHANGES.txt URL: http://svn.apache.org/viewvc/nutch/trunk/CHANGES.txt?rev=1560985&r1=1560984&r2=1560985&view=diff ============================================================================== --- nutch/trunk/CHANGES.txt (original) +++ nutch/trunk/CHANGES.txt Fri Jan 24 13:12:00 2014 @@ -2,6 +2,8 @@ Nutch Change Log Nutch Development Trunk +* NUTCH-356 Plugin repository cache can lead to memory leak (Enrico Triolo, DoÄacan Güney via markus) + * NUTCH-1413 Record response time (Yasin Kılınç, Talat Uyarer, snagel) * NUTCH-1325 HostDB for Nutch (markus, tejasp) Modified: nutch/trunk/src/java/org/apache/nutch/plugin/Extension.java URL: http://svn.apache.org/viewvc/nutch/trunk/src/java/org/apache/nutch/plugin/Extension.java?rev=1560985&r1=1560984&r2=1560985&view=diff ============================================================================== --- nutch/trunk/src/java/org/apache/nutch/plugin/Extension.java (original) +++ nutch/trunk/src/java/org/apache/nutch/plugin/Extension.java Fri Jan 24 13:12:00 2014 @@ -33,7 +33,6 @@ public class Extension { private String fClazz; private HashMap<String, String> fAttributes; private Configuration conf; - private PluginRepository pluginRepository; /** * @param pDescriptor @@ -52,7 +51,6 @@ public class Extension { setId(pId); setClazz(pExtensionClass); this.conf = conf; - this.pluginRepository = pluginRepository; } /** @@ -149,12 +147,13 @@ public class Extension { // The same is in PluginRepository.getPluginInstance(). // Suggested by Stefan Groschupf <s...@media-style.com> synchronized (getId()) { - try { - PluginClassLoader loader = fDescriptor.getClassLoader(); - Class<?> extensionClazz = loader.loadClass(getClazz()); + try { + PluginRepository pluginRepository = PluginRepository.get(conf); + Class extensionClazz = + pluginRepository.getCachedClass(fDescriptor, getClazz()); // lazy loading of Plugin in case there is no instance of the plugin // already. - this.pluginRepository.getPluginInstance(getDescriptor()); + pluginRepository.getPluginInstance(getDescriptor()); Object object = extensionClazz.newInstance(); if (object instanceof Configurable) { ((Configurable) object).setConf(this.conf); Modified: nutch/trunk/src/java/org/apache/nutch/plugin/PluginClassLoader.java URL: http://svn.apache.org/viewvc/nutch/trunk/src/java/org/apache/nutch/plugin/PluginClassLoader.java?rev=1560985&r1=1560984&r2=1560985&view=diff ============================================================================== --- nutch/trunk/src/java/org/apache/nutch/plugin/PluginClassLoader.java (original) +++ nutch/trunk/src/java/org/apache/nutch/plugin/PluginClassLoader.java Fri Jan 24 13:12:00 2014 @@ -18,6 +18,7 @@ package org.apache.nutch.plugin; import java.net.URL; import java.net.URLClassLoader; +import java.util.Arrays; /** * The <code>PluginClassLoader</code> contains only classes of the runtime @@ -30,6 +31,10 @@ import java.net.URLClassLoader; * @author joa23 */ public class PluginClassLoader extends URLClassLoader { + + private URL[] urls; + private ClassLoader parent; + /** * Construtor * @@ -40,5 +45,36 @@ public class PluginClassLoader extends U */ public PluginClassLoader(URL[] urls, ClassLoader parent) { super(urls, parent); + + this.urls = urls; + this.parent = parent; + } + + @Override + public int hashCode() { + final int PRIME = 31; + int result = 1; + result = PRIME * result + ((parent == null) ? 0 : parent.hashCode()); + result = PRIME * result + Arrays.hashCode(urls); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + final PluginClassLoader other = (PluginClassLoader) obj; + if (parent == null) { + if (other.parent != null) + return false; + } else if (!parent.equals(other.parent)) + return false; + if (!Arrays.equals(urls, other.urls)) + return false; + return true; } } Modified: nutch/trunk/src/java/org/apache/nutch/plugin/PluginRepository.java URL: http://svn.apache.org/viewvc/nutch/trunk/src/java/org/apache/nutch/plugin/PluginRepository.java?rev=1560985&r1=1560984&r2=1560985&view=diff ============================================================================== --- nutch/trunk/src/java/org/apache/nutch/plugin/PluginRepository.java (original) +++ nutch/trunk/src/java/org/apache/nutch/plugin/PluginRepository.java Fri Jan 24 13:12:00 2014 @@ -53,6 +53,9 @@ public class PluginRepository { private HashMap<String, ExtensionPoint> fExtensionPoints; private HashMap<String, Plugin> fActivatedPlugins; + + private static final Map<String, Map<PluginClassLoader, Class>> CLASS_CACHE = + new HashMap<String, Map<PluginClassLoader,Class>>(); private Configuration conf; @@ -65,10 +68,10 @@ public class PluginRepository { public PluginRepository(Configuration conf) throws RuntimeException { fActivatedPlugins = new HashMap<String, Plugin>(); fExtensionPoints = new HashMap<String, ExtensionPoint>(); - this.conf = conf; + this.conf = new Configuration(conf); this.auto = conf.getBoolean("plugin.auto-activation", true); String[] pluginFolders = conf.getStrings("plugin.folders"); - PluginManifestParser manifestParser = new PluginManifestParser(conf, this); + PluginManifestParser manifestParser = new PluginManifestParser(this.conf, this); Map<String, PluginDescriptor> allPlugins = manifestParser .parsePluginFolder(pluginFolders); Pattern excludes = Pattern.compile(conf.get("plugin.excludes", "")); @@ -267,8 +270,7 @@ public class PluginRepository { // The same is in Extension.getExtensionInstance(). // Suggested by Stefan Groschupf <s...@media-style.com> synchronized (pDescriptor) { - PluginClassLoader loader = pDescriptor.getClassLoader(); - Class<?> pluginClass = loader.loadClass(pDescriptor.getPluginClass()); + Class<?> pluginClass = getCachedClass(pDescriptor, pDescriptor.getPluginClass()); Constructor<?> constructor = pluginClass.getConstructor(new Class<?>[] { PluginDescriptor.class, Configuration.class }); Plugin plugin = (Plugin) constructor.newInstance(new Object[] { @@ -296,7 +298,7 @@ public class PluginRepository { * @see java.lang.Object#finalize() */ public void finalize() throws Throwable { - shotDownActivatedPlugins(); + shutDownActivatedPlugins(); } /** @@ -304,11 +306,27 @@ public class PluginRepository { * * @throws PluginRuntimeException */ - private void shotDownActivatedPlugins() throws PluginRuntimeException { + private void shutDownActivatedPlugins() throws PluginRuntimeException { for (Plugin plugin : fActivatedPlugins.values()) { plugin.shutDown(); } } + + public Class getCachedClass(PluginDescriptor pDescriptor, String className) + throws ClassNotFoundException { + Map<PluginClassLoader, Class> descMap = CLASS_CACHE.get(className); + if (descMap == null) { + descMap = new HashMap<PluginClassLoader, Class>(); + CLASS_CACHE.put(className, descMap); + } + PluginClassLoader loader = pDescriptor.getClassLoader(); + Class clazz = descMap.get(loader); + if (clazz == null) { + clazz = loader.loadClass(className); + descMap.put(loader, clazz); + } + return clazz; + } private void displayStatus() { LOG.info("Plugin Auto-activation mode: [" + this.auto + "]");