/hg/icedtea-web: Resolve multiple-applet deadlock issue in JNLPC...
aazores at icedtea.classpath.org
aazores at icedtea.classpath.org
Wed Oct 16 10:13:57 PDT 2013
changeset 744442d54cbf in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=744442d54cbf
author: Andrew Azores <aazores at redhat.com>
date: Wed Oct 16 13:13:19 2013 -0400
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
diffstat:
ChangeLog | 18 +
netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java | 143 +++++----
tests/reproducers/custom/JNLPClassLoaderDeadlock/resources/JNLPClassLoaderDeadlock.html | 7 +
tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_1.java | 24 +
tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_2.java | 83 +++++
tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/Makefile | 30 ++
tests/reproducers/custom/JNLPClassLoaderDeadlock/testcases/JNLPClassLoaderDeadlockTest.java | 70 ++++
7 files changed, 309 insertions(+), 66 deletions(-)
diffs (435 lines):
diff -r 60b47a6ec2e4 -r 744442d54cbf ChangeLog
--- a/ChangeLog Fri Oct 11 10:31:55 2013 -0400
+++ b/ChangeLog Wed Oct 16 13:13:19 2013 -0400
@@ -1,3 +1,21 @@
+2013-10-16 Andrew Azores <aazores at redhat.com>
+
+ 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 <aazores at redhat.com>
* netx/net/sourceforge/jnlp/security/SecurityDialog.java: (initDialog)
diff -r 60b47a6ec2e4 -r 744442d54cbf netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
--- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Fri Oct 11 10:31:55 2013 -0400
+++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Wed Oct 16 13:13:19 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.
*
@@ -1410,88 +1420,89 @@
* 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
- // 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
+ 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);
+ }
- // 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);
+ result = loadClassExt(name);
+ return result;
+ } catch (ClassNotFoundException cnfe1) {
+ OutputController.getLogger().log(cnfe1);
}
- result = loadClassExt(name);
- return result;
- } catch (ClassNotFoundException cnfe1) {
- OutputController.getLogger().log(cnfe1);
- }
+ // 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<String> 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<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 (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 it still fails, let it error out
- result = loadClassExt(name);
}
}
}
+
+ if (result == null) {
+ throw new ClassNotFoundException(name);
+ }
+
+ return result;
}
-
- if (result == null) {
- throw new ClassNotFoundException(name);
- }
-
- return result;
}
/**
diff -r 60b47a6ec2e4 -r 744442d54cbf 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 Wed Oct 16 13:13:19 2013 -0400
@@ -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 -r 60b47a6ec2e4 -r 744442d54cbf 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 Wed Oct 16 13:13:19 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 60b47a6ec2e4 -r 744442d54cbf 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 Wed Oct 16 13:13:19 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 60b47a6ec2e4 -r 744442d54cbf tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/Makefile
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/Makefile Wed Oct 16 13:13:19 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 60b47a6ec2e4 -r 744442d54cbf 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 Wed Oct 16 13:13:19 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"));
+ }
+
+}
More information about the distro-pkg-dev
mailing list