[icedtea-web] RFC: Patch to support plugin classloader sharing
Deepak Bhole
dbhole at redhat.com
Fri Mar 4 12:55:28 PST 2011
* Dr Andrew John Hughes <ahughes at redhat.com> [2011-03-03 07:41]:
> On 21:08 Wed 02 Mar , Deepak Bhole wrote:
> > > > >
<snip>
> > > >
> > > > This is horrible. Not only is it going to break if the implementation details of
> > > > URLClassLoader change (have you checked OpenJDK7?) but it will fail with any
> > > > different implementation of URLClassLoader. Can you not override a method or
> > > > use our own classloader instead of this awful hack?
> > > >
> > >
> > > Yep, I agree 100% .. (well 99.9 :)) that it is a terrible hack.
> > >
<snip>
> > >
> >
> > Hold the reply please. I just thought of another solution that might
> > work, but it is more disruptive (from code perspective). I will
> > implement it and post it tomorrow and we can discuss which one to adopt
> > on the list.
> >
>
> Ok. I'd pretty much prefer most things to this solution.
>
Fair enough :) I realized after sending the original patch that the
ideas of the new design could be implemented (at a higher level) into
the existing one. So here is a patch that does not use any reflection,
nor does it add any additional sun.misc dependencies.
Though this may appear like a huge change, I have tried hard to keep it
very simple, and have tested it as much as possible. I think it is safe
for 1.0 as well as HEAD though I will forgo 1.0 if the reviewer thinks
it is unsafe.
ChangeLog:
2011-03-04 Deepak Bhole <dbhole at redhat.com>
* NEWS: Updated.
* netx/net/sourceforge/jnlp/PluginBridge.java (PluginBridge): Use
documentbase as a uniquekey so that the classloader may be shared by
applets from the same page.
* netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java: Added new
CodeBaseClassLoader class to load codebase (from path instead of a file)
classes.
(getInstance): Try to match file locations only for Web Start apps. For
plugin, merge the new loader into current one.
(enableCodeBase): Use the new addToCodeBaseLoader method.
(findLoadedClassAll): Search the codebase loader if the class was not
found in the file loaders.
(findClass): Likewise.
(getResource): Likewise.
(findResources): Likewise.
(merge): Merge codebase loaders.
(addToCodeBaseLoader): New method. Adds a given url to the codebase loader
if it is a path.
(CodeBaseClassLoader): New inner class. Extends URLClassLoader to expose
its protected methods like addURL.
* netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java
(getApplication): Accomodate the fact that the classloader for a class may
be a CodeBaseClassLoader.
* plugin/icedteanp/java/sun/applet/PluginAppletViewer.java (run):
Likewise.
Cheers,
Deepak
-------------- next part --------------
diff -r 64b9d3a8239c NEWS
--- a/NEWS Thu Mar 03 17:56:00 2011 -0500
+++ b/NEWS Fri Mar 04 15:46:36 2011 -0500
@@ -21,6 +21,7 @@
- Use Firefox's proxy settings if possible
- RH669942: javaws fails to download version/packed files (missing support for jnlp.packEnabled and jnlp.versionEnabled)
* Plugin
+ - PR475, RH604061: Allow applets from the same page to use the same classloader
- PR612: NetDania application ends on java.security.AccessControlException: access denied (java.util.PropertyPermission browser read)
New in release 1.0 (2010-XX-XX):
diff -r 64b9d3a8239c netx/net/sourceforge/jnlp/PluginBridge.java
--- a/netx/net/sourceforge/jnlp/PluginBridge.java Thu Mar 03 17:56:00 2011 -0500
+++ b/netx/net/sourceforge/jnlp/PluginBridge.java Fri Mar 04 15:46:36 2011 -0500
@@ -130,9 +130,10 @@
else
security = null;
- this.uniqueKey = Calendar.getInstance().getTimeInMillis() + "-" +
- Math.abs(((new java.util.Random()).nextInt())) + "-" +
- documentBase;
+ // Plugin needs to share classloaders so that applet instances from
+ // same page can communicate (there are applets known to require
+ // such communication for proper functionality)
+ this.uniqueKey = documentBase.toString();
}
public String getTitle() {
diff -r 64b9d3a8239c netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
--- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Thu Mar 03 17:56:00 2011 -0500
+++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Fri Mar 04 15:46:36 2011 -0500
@@ -145,6 +145,9 @@
/** Map of specific codesources to securitydesc */
private HashMap<URL, SecurityDesc> jarLocationSecurityMap =
new HashMap<URL, SecurityDesc>();
+
+ /** Loader for codebase (which is a path, rather than a file) */
+ private CodeBaseClassLoader codeBaseLoader;
/**
* Create a new JNLPClassLoader from the specified file.
@@ -276,10 +279,12 @@
try {
- // If base loader is null, or the baseloader's file and this
- // file is different, initialize a new loader
+
+ // A null baseloader implies that no loader has been created
+ // for this codebase/jnlp yet. Create one.
if (baseLoader == null ||
- !baseLoader.getJNLPFile().getFileLocation().equals(file.getFileLocation())) {
+ (file.isApplication() &&
+ !baseLoader.getJNLPFile().getFileLocation().equals(file.getFileLocation()))) {
loader = new JNLPClassLoader(file, policy);
@@ -303,6 +308,13 @@
} else {
// if key is same and locations match, this is the loader we want
+ if (!file.isApplication()) {
+ // If this is an applet, we do need to consider its loader
+ loader = new JNLPClassLoader(file, policy);
+
+ if (baseLoader != null)
+ baseLoader.merge(loader);
+ }
loader = baseLoader;
}
@@ -529,7 +541,7 @@
* loaded from the codebase are not cached.
*/
public void enableCodeBase() {
- addURL(file.getCodeBase()); // nothing happens if called more that once?
+ addToCodeBaseLoader(file.getCodeBase());
}
/**
@@ -958,8 +970,13 @@
if (result != null)
return result;
}
-
- return null;
+
+ // Result is still null. Return what the codebaseloader
+ // has (which returns null if it is not loaded there either)
+ if (codeBaseLoader != null)
+ return codeBaseLoader.findLoadedClassFromParent(name);
+ else
+ return null;
}
/**
@@ -1067,6 +1084,11 @@
}
}
+ // Try codebase loader
+ if (codeBaseLoader != null)
+ return codeBaseLoader.findClass(name);
+
+ // All else failed. Throw CNFE
throw new ClassNotFoundException(name);
}
@@ -1109,6 +1131,10 @@
for (int i = 1; i < loaders.length; i++)
if (result == null)
result = loaders[i].getResource(name);
+
+ // If result is still null, look in the codebase loader
+ if (result == null && codeBaseLoader != null)
+ result = codeBaseLoader.getResource(name);
return result;
}
@@ -1120,9 +1146,9 @@
@Override
public Enumeration<URL> findResources(String name) throws IOException {
Vector<URL> resources = new Vector<URL>();
+ Enumeration<URL> e;
for (int i = 0; i < loaders.length; i++) {
- Enumeration<URL> e;
if (loaders[i] == this)
e = super.findResources(name);
@@ -1133,6 +1159,14 @@
resources.add(e.nextElement());
}
+ // Add resources from codebase (only if nothing was found above,
+ // otherwise the server will get hammered)
+ if (resources.isEmpty() && codeBaseLoader != null) {
+ e = codeBaseLoader.findResources(name);
+ while (e.hasMoreElements())
+ resources.add(e.nextElement());
+ }
+
return resources.elements();
}
@@ -1250,6 +1284,9 @@
// jars
for (URL u : extLoader.getURLs())
addURL(u);
+
+ // Codebase
+ addToCodeBaseLoader(extLoader.file.getCodeBase());
// native search paths
for (File nativeDirectory : extLoader.getNativeDirectories())
@@ -1261,6 +1298,28 @@
}
}
+ /**
+ * Adds the given path to the path loader
+ *
+ * @param URL the path to add
+ * @throws IllegalArgumentException If the given url is not a path
+ */
+ private void addToCodeBaseLoader(URL u) {
+
+ // Only paths may be added
+ if (!u.getFile().endsWith("/")) {
+ throw new IllegalArgumentException("addToPathLoader only accepts path based URLs");
+ }
+
+ // If there is no loader yet, create one, else add it to the
+ // existing one (happens when called from merge())
+ if (codeBaseLoader == null) {
+ codeBaseLoader = new CodeBaseClassLoader(new URL[] { u }, this);
+ } else {
+ codeBaseLoader.addURL(u);
+ }
+ }
+
private DownloadOptions getDownloadOptionsForJar(JARDesc jar) {
boolean usePack = false;
boolean useVersion = false;
@@ -1283,4 +1342,49 @@
return new DownloadOptions(usePack, useVersion);
}
+ /*
+ * Helper class to expose protected URLClassLoader methods.
+ */
+
+ public class CodeBaseClassLoader extends URLClassLoader {
+
+ JNLPClassLoader parentJNLPClassLoader;
+
+ public CodeBaseClassLoader(URL[] urls, JNLPClassLoader cl) {
+ super(urls);
+ parentJNLPClassLoader = cl;
+ }
+
+ @Override
+ public void addURL(URL url) {
+ super.addURL(url);
+ }
+
+ @Override
+ public Class<?> findClass(String name) throws ClassNotFoundException {
+ return super.findClass(name);
+ }
+
+ /**
+ * Returns the output of super.findLoadedClass().
+ *
+ * The method is renamed because ClassLoader.findLoadedClass() is final
+ *
+ * @param name The name of the class to find
+ * @return Output of ClassLoader.findLoadedClass() which is the class if found, null otherwise
+ * @see java.lang.ClassLoader#findLoadedClass(String)
+ */
+ public Class<?> findLoadedClassFromParent(String name) {
+ return findLoadedClass(name);
+ }
+
+ /**
+ * Returns JNLPClassLoader that encompasses this loader
+ *
+ * @return parent JNLPClassLoader
+ */
+ public JNLPClassLoader getParentJNLPClassLoader() {
+ return parentJNLPClassLoader;
+ }
+ }
}
diff -r 64b9d3a8239c netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java
--- a/netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java Thu Mar 03 17:56:00 2011 -0500
+++ b/netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java Fri Mar 04 15:46:36 2011 -0500
@@ -199,8 +199,16 @@
// this needs to be tightened up
for (int i = 0; i < stack.length && i < maxDepth; i++) {
- if (stack[i].getClassLoader() instanceof JNLPClassLoader) {
- JNLPClassLoader loader = (JNLPClassLoader) stack[i].getClassLoader();
+ ClassLoader cl = stack[i].getClassLoader();
+
+ // Since we want to deal with JNLPClassLoader, extract it if this
+ // is a codebase loader
+ if (cl instanceof JNLPClassLoader.CodeBaseClassLoader)
+ cl = ((JNLPClassLoader.CodeBaseClassLoader) cl).getParentJNLPClassLoader();
+
+ if (cl instanceof JNLPClassLoader) {
+
+ JNLPClassLoader loader = (JNLPClassLoader) cl;
if (loader != null && loader.getApplication() != null) {
return loader.getApplication();
diff -r 64b9d3a8239c plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
--- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Thu Mar 03 17:56:00 2011 -0500
+++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Fri Mar 04 15:46:36 2011 -0500
@@ -1646,7 +1646,14 @@
{
public void run()
{
- ThreadGroup tg = ((JNLPClassLoader) p.applet.getClass().getClassLoader()).getApplication().getThreadGroup();
+ ClassLoader cl = p.applet.getClass().getClassLoader();
+
+ // Since we want to deal with JNLPClassLoader, extract it if this
+ // is a codebase loader
+ if (cl instanceof JNLPClassLoader.CodeBaseClassLoader)
+ cl = ((JNLPClassLoader.CodeBaseClassLoader) cl).getParentJNLPClassLoader();
+
+ ThreadGroup tg = ((JNLPClassLoader) cl).getApplication().getThreadGroup();
appletShutdown(p);
appletPanels.removeElement(p);
More information about the distro-pkg-dev
mailing list