# HG changeset patch # User Andrew Azores # Date 1382022591 14400 # Node ID 29c1c1f92e09e1c789ae637f2a2fc88162beaab4 # Parent c43d49d0db977ce72efd94c0e124ffd7c62a527e Resolve multiple-applet deadlock issue in JNLPClassLoader New lock used for synchronizing JNLPClassLoader#loadClass(String) to avoid deadlock condition when multiple applets are being loaded simultaneously. Regression test included. * 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 diff -r c43d49d0db97 -r 29c1c1f92e09 ChangeLog --- a/ChangeLog Fri Oct 11 10:39:07 2013 -0400 +++ b/ChangeLog Thu Oct 17 11:09:51 2013 -0400 @@ -1,3 +1,21 @@ +2013-10-16 Andrew Azores + + 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 * netx/net/sourceforge/jnlp/security/SecurityDialog.java: (initDialog) diff -r c43d49d0db97 -r 29c1c1f92e09 NEWS --- a/NEWS Fri Oct 11 10:39:07 2013 -0400 +++ b/NEWS Thu Oct 17 11:09:51 2013 -0400 @@ -9,6 +9,9 @@ CVE-XXXX-YYYY: http://www.cve.mitre.org/cgi-bin/cvename.cgi?name=XXXX-YYYY New in release 1.4.2 (2013-MM-DD): +* Dialogs center on screen before becoming visible +* Plugin + - RH976833: Multiple applets on one page cause deadlock New in release 1.4.1 (2013-09-19): * Improved and cleaned Temporary internet files panel diff -r c43d49d0db97 -r 29c1c1f92e09 netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Fri Oct 11 10:39:07 2013 -0400 +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Thu Oct 17 11:09:51 2013 -0400 @@ -205,6 +205,16 @@ */ 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. * @@ -1542,92 +1552,93 @@ * classloader, or one of the classloaders for the JNLP file's * extensions. */ - public synchronized Class loadClass(String name) throws ClassNotFoundException { - - Class result = findLoadedClassAll(name); + public Class loadClass(String name) throws ClassNotFoundException { + synchronized (loadClassLock) { + 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); + result = loadClassExt(name); + } catch (ClassNotFoundException cnfe) { + // Not found in external loader either + + // Look in 'Class-Path' as specified in the manifest file + 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); } - addNewJar(desc); + + result = loadClassExt(name); + return result; + } catch (ClassNotFoundException cnfe1) { + if (JNLPRuntime.isDebug()) { + cnfe1.printStackTrace(); + } } - result = loadClassExt(name); - return result; - } catch (ClassNotFoundException cnfe1) { - if (JNLPRuntime.isDebug()) { - cnfe1.printStackTrace(); - } - } + // As a last resort, look in any available indexes - // 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 jarList = index.get(name.replace('.', '/')); - // 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 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) { - if (JNLPRuntime.isDebug()) { - e.printStackTrace(); + 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) { + if (JNLPRuntime.isDebug()) { + e.printStackTrace(); + } } } + + // If it still fails, let it error out + result = loadClassExt(name); } - - // 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 -r c43d49d0db97 -r 29c1c1f92e09 tests/reproducers/custom/JNLPClassLoaderDeadlock/resources/JNLPClassLoaderDeadlock.html --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/resources/JNLPClassLoaderDeadlock.html Thu Oct 17 11:09:51 2013 -0400 @@ -0,0 +1,7 @@ + + + +

+

+ + diff -r c43d49d0db97 -r 29c1c1f92e09 tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_1.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_1.java Thu Oct 17 11:09:51 2013 -0400 @@ -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 -r c43d49d0db97 -r 29c1c1f92e09 tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_2.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_2.java Thu Oct 17 11:09:51 2013 -0400 @@ -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 -r c43d49d0db97 -r 29c1c1f92e09 tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/Makefile --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/Makefile Thu Oct 17 11:09:51 2013 -0400 @@ -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 -r c43d49d0db97 -r 29c1c1f92e09 tests/reproducers/custom/JNLPClassLoaderDeadlock/testcases/JNLPClassLoaderDeadlockTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/testcases/JNLPClassLoaderDeadlockTest.java Thu Oct 17 11:09:51 2013 -0400 @@ -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")); + } + +}