changeset 762:29c1c1f92e09

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
author Andrew Azores <aazores@redhat.com>
date Thu, 17 Oct 2013 11:09:51 -0400
parents c43d49d0db97
children eafa63cec4ca
files ChangeLog NEWS netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java tests/reproducers/custom/JNLPClassLoaderDeadlock/resources/JNLPClassLoaderDeadlock.html tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_1.java tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/JNLPClassLoaderDeadlock_2.java tests/reproducers/custom/JNLPClassLoaderDeadlock/srcs/Makefile tests/reproducers/custom/JNLPClassLoaderDeadlock/testcases/JNLPClassLoaderDeadlockTest.java
diffstat 8 files changed, 313 insertions(+), 67 deletions(-) [+]
line wrap: on
line diff
--- 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  <aazores@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@redhat.com>
 
 	* netx/net/sourceforge/jnlp/security/SecurityDialog.java: (initDialog)
--- 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
--- 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<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) {
-                                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;
+            }
     }
 
     /**
--- /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 @@
+<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>
--- /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();
+    }
+}
--- /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 ***");
+    }
+}
--- /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)
--- /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"));
+    }
+
+}