Mercurial > hg > release > icedtea-web-1.3

changeset 514:88fb945c9397

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix PR580: http://www.horaoficial.cl/ loads improperly. Applets that must share a class-loader now load sequentially.
author Jiri Vanek <jvanek@redhat.com>
date Thu, 11 Apr 2013 15:45:27 +0200
parents 25dd7c7ac39c
children 2d76719a5e4d
files ChangeLog NEWS netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
diffstat 3 files changed, 78 insertions(+), 31 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Thu Apr 11 12:29:47 2013 +0200
+++ b/ChangeLog	Thu Apr 11 15:45:27 2013 +0200
@@ -1,3 +1,17 @@
+2013-04-11  Adam Domurad  <adomurad@redhat.com>
+
+	Fix PR580: http://www.horaoficial.cl/ loads improperly. Applets that
+	must share a class-loader now load sequentially.
+	* NEWS:
+	Mention the fix.
+	* netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
+	(getUniqueKeyLock): New, atomically grabs or creates a lock for the
+	unique key.
+	(getInstance): Ensure classloader initialization is locked by unique
+	key.
+	(decrementLoaderUseCount): Ensure classloader deinitialization is
+	locked by unique key, get rid of no-longer used locks. 
+
 2013-04-11 Adam Domurad  <adomurad@redhat.com>
 
 	CVE-2013-1926, RH916774: Class-loader incorrectly shared for applets with
--- a/NEWS	Thu Apr 11 12:29:47 2013 +0200
+++ b/NEWS	Thu Apr 11 15:45:27 2013 +0200
@@ -14,6 +14,8 @@
   - CVE-2013-1926, RH916774: Class-loader incorrectly shared for applets with same relative-path.
 * Common
   - Added new option in itw-settings which allows users to set JVM arguments when plugin is initialized.
+* NetX
+  - PR580: http://www.horaoficial.cl/ loads improperly
 * Plugin
    PR1260: IcedTea-Web should not rely on GTK
    PR1157: Applets can hang browser after fatal exception
--- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java	Thu Apr 11 12:29:47 2013 +0200
+++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java	Thu Apr 11 15:45:27 2013 +0200
@@ -17,6 +17,10 @@
 
 import static net.sourceforge.jnlp.runtime.Translator.R;
 
+import java.util.concurrent.locks.ReentrantLock;
+
+import java.util.concurrent.locks.Lock;
+
 import java.io.Closeable;
 import java.io.File;
 import java.io.FileOutputStream;
@@ -106,9 +110,12 @@
     /** True if the application has a signed JNLP File */
     private boolean isSignedJNLP = false;
     
-    /** map from JNLPFile url to shared classloader */
-    private static Map<String, JNLPClassLoader> urlToLoader =
-            new HashMap<String, JNLPClassLoader>(); // never garbage collected!
+    /** map from JNLPFile unique key to shared classloader */
+    private static Map<String, JNLPClassLoader> uniqueKeyToLoader = new ConcurrentHashMap<String, JNLPClassLoader>();
+
+    /** map from JNLPFile unique key to lock, the lock is needed to enforce correct 
+     * initialization of applets that share a unique key*/
+    private static Map<String, ReentrantLock> uniqueKeyToLock = new HashMap<String, ReentrantLock>();
 
     /** the directory for native code */
     private File nativeDir = null; // if set, some native code exists
@@ -331,6 +338,26 @@
     }
 
     /**
+     * Gets the lock for a given unique key, creating one if it does not yet exist.
+     * This operation is atomic & thread-safe.
+     * 
+     * @param file the file whose unique key should be used
+     * @return the lock
+     */
+    private static ReentrantLock getUniqueKeyLock(String uniqueKey) {
+        synchronized (uniqueKeyToLock) {
+            ReentrantLock storedLock = uniqueKeyToLock.get(uniqueKey);
+
+            if (storedLock == null) {
+                storedLock = new ReentrantLock();
+                uniqueKeyToLock.put(uniqueKey, storedLock);
+            }
+
+            return storedLock;
+        }
+    }
+
+    /**
      * Returns a JNLP classloader for the specified JNLP file.
      *
      * @param file the file to load classes for
@@ -342,11 +369,8 @@
         JNLPClassLoader loader = null;
         String uniqueKey = file.getUniqueKey();
 
-        if (uniqueKey != null)
-            baseLoader = urlToLoader.get(uniqueKey);
-
-        try {
-
+        synchronized ( getUniqueKeyLock(uniqueKey) ) {
+            baseLoader = uniqueKeyToLoader.get(uniqueKey);
 
             // A null baseloader implies that no loader has been created 
             // for this codebase/jnlp yet. Create one.
@@ -358,7 +382,7 @@
 
                 // New loader init may have caused extentions to create a
                 // loader for this unique key. Check.
-                JNLPClassLoader extLoader = urlToLoader.get(uniqueKey);
+                JNLPClassLoader extLoader = uniqueKeyToLoader.get(uniqueKey);
 
                 if (extLoader != null && extLoader != loader) {
                     if (loader.signing && !extLoader.signing)
@@ -387,16 +411,12 @@
                 loader = baseLoader;
             }
 
-        } catch (LaunchException e) {
-            throw e;
-        }
+            // loaders are mapped to a unique key. Only extensions and parent
+            // share a key, so it is safe to always share based on it
 
-        // loaders are mapped to a unique key. Only extensions and parent
-        // share a key, so it is safe to always share based on it
-        
-        loader.incrementLoaderUseCount();
-        synchronized(urlToLoader) {
-            urlToLoader.put(uniqueKey, loader);
+            loader.incrementLoaderUseCount();
+
+            uniqueKeyToLoader.put(uniqueKey, loader);
         }
 
         return loader;
@@ -413,12 +433,17 @@
      */
     public static JNLPClassLoader getInstance(URL location, String uniqueKey, Version version, UpdatePolicy policy, String mainName)
             throws IOException, ParseException, LaunchException {
-        JNLPClassLoader loader = urlToLoader.get(uniqueKey);
+
+        JNLPClassLoader loader;
+
+        synchronized ( getUniqueKeyLock(uniqueKey) ) {
+            loader = uniqueKeyToLoader.get(uniqueKey);
 
-        if (loader == null || !location.equals(loader.getJNLPFile().getFileLocation())) {
-            JNLPFile jnlpFile = new JNLPFile(location, uniqueKey, version, false, policy);
+            if (loader == null || !location.equals(loader.getJNLPFile().getFileLocation())) {
+                JNLPFile jnlpFile = new JNLPFile(location, uniqueKey, version, false, policy);
 
-            loader = getInstance(jnlpFile, policy, mainName);
+                loader = getInstance(jnlpFile, policy, mainName);
+            }
         }
 
         return loader;
@@ -2097,13 +2122,16 @@
      * 
      * @throws SecurityException if caller is not trusted
      */
-    private synchronized void incrementLoaderUseCount() {
-        
+    private void incrementLoaderUseCount() {
+
         // For use by trusted code only
         if (System.getSecurityManager() != null)
             System.getSecurityManager().checkPermission(new AllPermission());
-        
-        useCount++;
+
+        // NB: There will only ever be one class-loader per unique-key
+        synchronized ( getUniqueKeyLock(file.getUniqueKey()) ){
+            useCount++;
+        }
     }
 
     /**
@@ -2113,17 +2141,20 @@
      * 
      * @throws SecurityException if caller is not trusted
      */
-    public synchronized void decrementLoaderUseCount() {
+    public void decrementLoaderUseCount() {
 
         // For use by trusted code only
         if (System.getSecurityManager() != null)
             System.getSecurityManager().checkPermission(new AllPermission());
 
-        useCount--;
+        String uniqueKey = file.getUniqueKey();
 
-        if (useCount <= 0) {
-            synchronized(urlToLoader) {
-                urlToLoader.remove(file.getUniqueKey());
+        // NB: There will only ever be one class-loader per unique-key
+        synchronized ( getUniqueKeyLock(uniqueKey) ) {
+            useCount--;
+
+            if (useCount <= 0) {
+                uniqueKeyToLoader.remove(uniqueKey);
             }
         }
     }