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

7 files changed, 305 insertions, 62 deletions

diff --git a/ChangeLog b/ChangeLog
index a35d4b3..b2a20b8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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"));
+    }
+
+}