diff options
7 files changed, 305 insertions, 62 deletions
@@ -1,3 +1,21 @@ +2013-10-16 Andrew Azores <[email protected]> + + Resolve deadlock issue when multiple applets are loaded simultaneously + (RH976833) + * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java: (loadClassLock) + private member for locking of loadClass method. (loadClass) synchronizes + using new lock rather than instance intrinsic lock to avoid RH976833 + deadlock + * tests/reproducers/custom/JNLPClassLoaderDeadlock/testcases/JNLPClassLoaderDeadlockTest.java: + new test for multiple applet deadlock condition + * tests/reproducers/custom/JNLPClassLoaderDeadlock/resources/JNLPClassLoaderDeadlock.html: + same + * tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_1.java: + same + * tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_2.java: + same + * tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/Makefile: same + 2013-10-11 Andrew Azores <[email protected]> * netx/net/sourceforge/jnlp/security/SecurityDialog.java: (initDialog) diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java index c387b35..e7a080e 100644 --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java @@ -205,6 +205,16 @@ public class JNLPClassLoader extends URLClassLoader { */ private int useCount = 0; + /* This Object is used as a lock on the loadClass(String) method. This method should not + * be entered by multiple threads simultaneously. This Object should be used for no other + * purpose than synchronizing the body of the loadClass(String) method. The intrinsic instance + * lock is not suitable for this purpose or else deadlock may occur. + * + * See bug RH976833 and the mailing list archive discussion: + * http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2013-September/024536.html + */ + private final Object loadClassLock = new Object(); + /** * Create a new JNLPClassLoader from the specified file. * @@ -1410,88 +1420,89 @@ public class JNLPClassLoader extends URLClassLoader { * classloader, or one of the classloaders for the JNLP file's * extensions. */ - public synchronized Class<?> loadClass(String name) throws ClassNotFoundException { + public Class<?> loadClass(String name) throws ClassNotFoundException { + synchronized (loadClassLock) { + Class<?> result = findLoadedClassAll(name); - Class<?> result = findLoadedClassAll(name); - - // try parent classloader - if (result == null) { - try { - ClassLoader parent = getParent(); - if (parent == null) - parent = ClassLoader.getSystemClassLoader(); + // try parent classloader + if (result == null) { + try { + ClassLoader parent = getParent(); + if (parent == null) + parent = ClassLoader.getSystemClassLoader(); - return parent.loadClass(name); - } catch (ClassNotFoundException ex) { + return parent.loadClass(name); + } catch (ClassNotFoundException ex) { + } } - } - // filter out 'bad' package names like java, javax - // validPackage(name); + // filter out 'bad' package names like java, javax + // validPackage(name); - // search this and the extension loaders - if (result == null) { - try { - result = loadClassExt(name); - } catch (ClassNotFoundException cnfe) { - // Not found in external loader either - - // Look in 'Class-Path' as specified in the manifest file + // search this and the extension loaders + if (result == null) { try { - for (String classpath: classpaths) { - JARDesc desc; - try { - URL jarUrl = new URL(file.getCodeBase(), classpath); - desc = new JARDesc(jarUrl, null, null, false, true, false, true); - } catch (MalformedURLException mfe) { - throw new ClassNotFoundException(name, mfe); - } - addNewJar(desc); - } - result = loadClassExt(name); - return result; - } catch (ClassNotFoundException cnfe1) { - OutputController.getLogger().log(cnfe1); - } - - // As a last resort, look in any available indexes - - // Currently this loads jars directly from the site. We cannot cache it because this - // call is initiated from within the applet, which does not have disk read/write permissions - for (JarIndex index : jarIndexes) { - // Non-generic code in sun.misc.JarIndex - @SuppressWarnings("unchecked") - LinkedList<String> jarList = index.get(name.replace('.', '/')); + } catch (ClassNotFoundException cnfe) { + // Not found in external loader either - if (jarList != null) { - for (String jarName : jarList) { + // Look in 'Class-Path' as specified in the manifest file + try { + for (String classpath : classpaths) { JARDesc desc; try { - desc = new JARDesc(new URL(file.getCodeBase(), jarName), - null, null, false, true, false, true); + URL jarUrl = new URL(file.getCodeBase(), classpath); + desc = new JARDesc(jarUrl, null, null, false, true, false, true); } catch (MalformedURLException mfe) { - throw new ClassNotFoundException(name); - } - try { - addNewJar(desc); - } catch (Exception e) { - OutputController.getLogger().log(e); + throw new ClassNotFoundException(name, mfe); } + addNewJar(desc); } - // If it still fails, let it error out result = loadClassExt(name); + return result; + } catch (ClassNotFoundException cnfe1) { + OutputController.getLogger().log(cnfe1); + } + + // As a last resort, look in any available indexes + + // Currently this loads jars directly from the site. We cannot cache it because this + // call is initiated from within the applet, which does not have disk read/write permissions + for (JarIndex index : jarIndexes) { + // Non-generic code in sun.misc.JarIndex + @SuppressWarnings("unchecked") + LinkedList<String> jarList = index.get(name.replace('.', '/')); + + if (jarList != null) { + for (String jarName : jarList) { + JARDesc desc; + try { + desc = new JARDesc(new URL(file.getCodeBase(), jarName), + null, null, false, true, false, true); + } catch (MalformedURLException mfe) { + throw new ClassNotFoundException(name); + } + try { + addNewJar(desc); + } catch (Exception e) { + OutputController.getLogger().log(e); + } + } + + // If it still fails, let it error out + result = loadClassExt(name); + } } } } - } - if (result == null) { - throw new ClassNotFoundException(name); - } + if (result == null) { + throw new ClassNotFoundException(name); + } - return result; + return result; + } } /** diff --git a/tests/reproducers/custom/JNLPClassLoaderDeadlock/resources/JNLPClassLoaderDeadlock.html b/tests/reproducers/custom/JNLPClassLoaderDeadlock/resources/JNLPClassLoaderDeadlock.html new file mode 100644 index 0000000..3020b30 --- /dev/null +++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/resources/JNLPClassLoaderDeadlock.html @@ -0,0 +1,7 @@ +<html> +<head></head> +<body> +<p><applet code="JNLPClassLoaderDeadlock_1.class" codebase="." width="384" height="32"></applet></p> +<p><applet code="JNLPClassLoaderDeadlock_2.class" codebase="." width="512" height="512"></applet></p> +</body> +</html> diff --git a/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_1.java b/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_1.java new file mode 100644 index 0000000..616e5d3 --- /dev/null +++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_1.java @@ -0,0 +1,24 @@ +import java.applet.Applet; +import java.awt.*; + +public class JNLPClassLoaderDeadlock_1 extends Applet { + + @Override + public void init() { + System.out.println("JNLPClassLoaderDeadlock_1 applet initialized"); + final String version = System.getProperty("java.version") + " (" + System.getProperty("java.vm.version") + ")"; + final String vendor = System.getProperty("java.vendor"); + final TextField tf = new TextField(40); + tf.setText(version + " -- " + vendor); + tf.setEditable(false); + tf.setBackground(Color.white); + setBackground(Color.white); + add(tf); + System.out.println("JNLPClassLoaderDeadlock_1 applet finished"); + System.out.println("*** APPLET FINISHED ***"); + } + + public static void main(String[] args) { + new JNLPClassLoaderDeadlock_1().init(); + } +} diff --git a/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_2.java b/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_2.java new file mode 100644 index 0000000..6def405 --- /dev/null +++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_2.java @@ -0,0 +1,83 @@ +import java.applet.Applet; +import java.awt.*; + +import java.security.*; +import java.util.PropertyPermission; + +public class JNLPClassLoaderDeadlock_2 extends Applet implements Runnable { + + private static final String propertyNames[] = { + "java.version", + "java.vendor", + "java.vendor.url", + "java.home", + "java.vm.specification.version", + "java.vm.specification.vendor", + "java.vm.specification.name", + "java.vm.version", + "java.vm.name", + "java.vm.home", + "java.specification.version", + "java.specification.vendor", + "java.specification.name", + "java.class.version", + "java.class.path", + "os.name", + "os.arch", + "os.version", + "file.separator", + "path.separator", + "line.separator", + "user.home", + "user.name", + "user.dir", + }; + + private Label[] propertyValues; + + @Override + public void init() { + System.out.println("JNLPClassLoaderDeadlock_2 applet initialized"); + GridBagLayout gridbaglayout = new GridBagLayout(); + setLayout(gridbaglayout); + + GridBagConstraints leftColumn = new GridBagConstraints(); + leftColumn.anchor = 20; + leftColumn.ipadx = 16; + + GridBagConstraints rightColumn = new GridBagConstraints(); + rightColumn.fill = 2; + rightColumn.gridwidth = 0; + rightColumn.weightx = 1.0D; + + Label labels[] = new Label[propertyNames.length]; + propertyValues = new Label[propertyNames.length]; + final String preloadText = "..."; + + for (int i = 0; i < propertyNames.length; ++i) { + labels[i] = new Label(propertyNames[i]); + gridbaglayout.setConstraints(labels[i], leftColumn); + add(labels[i]); + + propertyValues[i] = new Label(preloadText); + gridbaglayout.setConstraints(propertyValues[i], rightColumn); + add(propertyValues[i]); + } + + Thread t = new Thread(this); + t.start(); + } + + @Override + public void run() { + for (int i = 0; i < propertyNames.length; ++i) { + try { + final String propertyValue = System.getProperty(propertyNames[i]); + propertyValues[i].setText(propertyValue); + } catch (SecurityException securityexception) { + } + } + System.out.println("JNLPClassLoaderDeadlock_2 applet finished"); + System.out.println("*** APPLET FINISHED ***"); + } +} diff --git a/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/Makefile b/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/Makefile new file mode 100644 index 0000000..15657da --- /dev/null +++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/Makefile @@ -0,0 +1,30 @@ +TESTNAME=JNLPClassLoaderDeadlock + +SRC_FILES=JNLPClassLoaderDeadlock_1.java JNLPClassLoaderDeadlock_2.java +RESOURCE_FILES=JNLPClassLoaderDeadlock.html +ENTRYPOINT_CLASSES=JNLPClassLoaderDeadlock_1 JNLPClassLoaderDeadlock_2 + +JAVAC_CLASSPATH=$(TEST_EXTENSIONS_DIR):$(NETX_DIR)/lib/classes.jar +JAVAC=$(BOOT_DIR)/bin/javac +JAR=$(BOOT_DIR)/bin/jar + +TMPDIR:=$(shell mktemp -d) + +prepare-reproducer: + echo PREPARING REPRODUCER $(TESTNAME) + + $(JAVAC) -d $(TMPDIR) -classpath $(JAVAC_CLASSPATH) $(SRC_FILES) + + cd ../resources; \ + cp $(RESOURCE_FILES) $(REPRODUCERS_TESTS_SERVER_DEPLOYDIR); \ + cd -; \ + for CLASS in $(ENTRYPOINT_CLASSES); \ + do \ + mv $(TMPDIR)/"$$CLASS.class" $(REPRODUCERS_TESTS_SERVER_DEPLOYDIR); \ + done; \ + + echo PREPARED REPRODUCER $(TESTNAME), removing $(TMPDIR) + rm -rf $(TMPDIR) + +clean-reproducer: + echo NOTHING TO CLEAN FOR $(TESTNAME) diff --git a/tests/reproducers/custom/JNLPClassLoaderDeadlock/testcases/JNLPClassLoaderDeadlockTest.java b/tests/reproducers/custom/JNLPClassLoaderDeadlock/testcases/JNLPClassLoaderDeadlockTest.java new file mode 100644 index 0000000..84b93d8 --- /dev/null +++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/testcases/JNLPClassLoaderDeadlockTest.java @@ -0,0 +1,70 @@ +/* JNLPClassLoaderDeadlockTest.java +Copyright (C) 2013 Red Hat, Inc. + +This file is part of IcedTea. + +IcedTea is free software; you can redistribute it and/or +modify it under the terms of the GNU General Public License as published by +the Free Software Foundation, version 2. + +IcedTea is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with IcedTea; see the file COPYING. If not, write to +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +02110-1301 USA. + +Linking this library statically or dynamically with other modules is +making a combined work based on this library. Thus, the terms and +conditions of the GNU General Public License cover the whole +combination. + +As a special exception, the copyright holders of this library give you +permission to link this library with independent modules to produce an +executable, regardless of the license terms of these independent +modules, and to copy and distribute the resulting executable under +terms of your choice, provided that you also meet, for each linked +independent module, the terms and conditions of the license of that +module. An independent module is a module which is not derived from +or based on this library. If you modify this library, you may extend +this exception to your version of the library, but you are not +obligated to do so. If you do not wish to do so, delete this +exception statement from your version. + */ + +import net.sourceforge.jnlp.ProcessResult; +import net.sourceforge.jnlp.ServerAccess.AutoClose; +import net.sourceforge.jnlp.annotations.Bug; +import net.sourceforge.jnlp.annotations.KnownToFail; +import net.sourceforge.jnlp.annotations.NeedsDisplay; +import net.sourceforge.jnlp.annotations.TestInBrowsers; +import net.sourceforge.jnlp.browsertesting.BrowserTest; +import net.sourceforge.jnlp.browsertesting.Browsers; +import net.sourceforge.jnlp.closinglisteners.AutoOkClosingListener; + +import static org.junit.Assert.assertTrue; +import org.junit.Test; + +public class JNLPClassLoaderDeadlockTest extends BrowserTest { + + @NeedsDisplay + @Test + @TestInBrowsers(testIn={Browsers.one}) + @Bug(id="RH976833") + public void testClassLoaderDeadlock() throws Exception { + ProcessResult pr = server.executeBrowser("JNLPClassLoaderDeadlock.html", AutoClose.CLOSE_ON_CORRECT_END); + assertTrue("First applet should have initialized", + pr.stdout.contains("JNLPClassLoaderDeadlock_1 applet initialized")); + assertTrue("Second applet should have initialized", + pr.stdout.contains("JNLPClassLoaderDeadlock_2 applet initialized")); + + assertTrue("First applet should have finished", + pr.stdout.contains("JNLPClassLoaderDeadlock_1 applet finished")); + assertTrue("Second applet should have finished", + pr.stdout.contains("JNLPClassLoaderDeadlock_2 applet finished")); + } + +} |