JNLPClassLoader.createInstance(): Remove extLoader (same as baseLoader); getInstance(): No new JNLPClassLoader if uniqueKey satisfies

JNLPClassLoader.[get|create]Instance(..):
  - Reuse uniqueKey, baseLoader from getInstance()

  - Remove extLoader (same as baseLoader)
    This is odd and sparked my eye while reusing the baseLoader (above).

JNLPClassLoader.getInstance(..):
  - No new JNLPClassLoader if uniqueKey satisfies.
    Why should we ? Enables reusing CL and impacts performance.

JNLPClassLoader.toString(): Added for better debugging.

1 files changed, 45 insertions, 18 deletions

diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
index 38cff29..85bdaa2 100644
--- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
+++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
@@ -378,14 +378,15 @@ public class JNLPClassLoader extends URLClassLoader {
      * In contrast, JNLP classloaders can also be constructed simply to merge
      * its resources into another classloader.
      *
+     * @param uniqueKey reuse file.getUniqueKey();
+     * @param baseLoader reuse uniqueKeyToLoader.get(uniqueKey);
      * @param file the file to load classes for
      * @param policy the update policy to use when downloading resources
      * @param mainName Overrides the main class name of the application
      */
-    private static JNLPClassLoader createInstance(JNLPFile file, UpdatePolicy policy, String mainName) throws LaunchException {
-        String uniqueKey = file.getUniqueKey();
-        JNLPClassLoader baseLoader = uniqueKeyToLoader.get(uniqueKey);
-        JNLPClassLoader loader = new JNLPClassLoader(file, policy, mainName);
+    private static JNLPClassLoader createInstance(final String uniqueKey, final JNLPClassLoader baseLoader,
+                                                  final JNLPFile file, final UpdatePolicy policy, final String mainName) throws LaunchException {
+        final JNLPClassLoader loader = new JNLPClassLoader(file, policy, mainName);
 
         // If security level is 'high' or greater, we must check if the user allows unsigned applets
         // when the JNLPClassLoader is created. We do so here, because doing so in the constructor
@@ -394,18 +395,21 @@ public class JNLPClassLoader extends URLClassLoader {
             UnsignedAppletTrustConfirmation.checkUnsignedWithUserIfRequired((PluginBridge)file);
         }
 
-        // New loader init may have caused extentions to create a
+        // New loader init may have caused extensions to create a
         // loader for this unique key. Check.
-        JNLPClassLoader extLoader = uniqueKeyToLoader.get(uniqueKey);
+        /**
+         * FIXME: I see no reason here, i.e. extLoader == baseLoader they are same!
+        final JNLPClassLoader extLoader = uniqueKeyToLoader.get(uniqueKey);
 
         if (extLoader != null && extLoader != loader) {
-            if (loader.getSigning() && !extLoader.getSigning())
-                if (!SecurityDialogs.showNotAllSignedWarningDialog(file))
+            if (loader.getSigning() && !extLoader.getSigning()) {
+                if (!SecurityDialogs.showNotAllSignedWarningDialog(file)) {
                     throw new LaunchException(file, null, R("LSFatal"), R("LCClient"), R("LSignedAppJarUsingUnsignedJar"), R("LSignedAppJarUsingUnsignedJarInfo"));
-
+                }
+            }
             loader.merge(extLoader);
             extLoader.decrementLoaderUseCount(); // loader urls have been merged, ext loader is no longer used
-        }
+        } */
 
         // loader is now current + ext. But we also need to think of
         // the baseLoader
@@ -426,6 +430,11 @@ public class JNLPClassLoader extends URLClassLoader {
         return getInstance(file, policy, null);
     }
 
+    @Override
+    public final String toString() {
+        final JNLPFile f = getJNLPFile();
+        return "JNLPClassLoader@"+hashCode()+"[refCnt "+useCount+", file "+f.getFileLocation()+", code "+f.getCodeBase()+"]";
+    }
     /**
      * Returns a JNLP classloader for the specified JNLP file.
      *
@@ -436,10 +445,15 @@ public class JNLPClassLoader extends URLClassLoader {
     public static JNLPClassLoader getInstance(JNLPFile file, UpdatePolicy policy, String mainName) throws LaunchException {
         JNLPClassLoader baseLoader = null;
         JNLPClassLoader loader = null;
-        String uniqueKey = file.getUniqueKey();
+        final String uniqueKey = file.getUniqueKey();
+        final OutputController out = OutputController.getLogger();
 
         synchronized ( getUniqueKeyLock(uniqueKey) ) {
             baseLoader = uniqueKeyToLoader.get(uniqueKey);
+            out.log("JNLPClassLoader.Cache: file isApp "+file.isApplication()+", file "+file.getFileLocation()+", code "+file.getCodeBase());
+            out.log("JNLPClassLoader.Cache: key "+uniqueKey +" -> "+baseLoader);
+            final int mode;
+            final boolean mapNewLoader;
 
             // A null baseloader implies that no loader has been created
             // for this codebase/jnlp yet. Create one.
@@ -447,25 +461,38 @@ public class JNLPClassLoader extends URLClassLoader {
                     (file.isApplication() &&
                      !baseLoader.getJNLPFile().getFileLocation().equals(file.getFileLocation()))) {
 
-                loader = createInstance(file, policy, mainName);
-            } else {
+                loader = createInstance(uniqueKey, baseLoader, file, policy, mainName);
+                mapNewLoader = true;
+                mode = 1;
+            } else { // baseLoader != null && ...
                 // if key is same and locations match, this is the loader we want
+                /***
+                 * The unique key is checked (codebase) - reuse!
+                 * Why reiterating ?
                 if (!file.isApplication()) {
                     // If this is an applet, we do need to consider its loader
                    loader = new JNLPClassLoader(file, policy, mainName);
-
-                    if (baseLoader != null)
-                        baseLoader.merge(loader);
+                   baseLoader.merge(loader);
+                   out.log("XXX JNLPClassLoader.2b: key "+uniqueKey +" -> "+baseLoader);
+                   mode = 3;
+                } else */ {
+                   mode = 2;
                 }
                 loader = baseLoader;
+                mapNewLoader = false;
             }
-
             // 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();
+            out.log("JNLPClassLoader.Cache: mode "+mode+", mapNewLoader "+mapNewLoader+", key "+uniqueKey +" -> "+loader);
 
-            uniqueKeyToLoader.put(uniqueKey, loader);
+            if( mapNewLoader ) {
+                synchronized(uniqueKeyMapLock) {
+                    uniqueKeyToLoaderActive.put(uniqueKey, loader);
+                }
+                out.log(new RuntimeException("JNLPClassLoader.Cache.added: "+loader));
+            }
         }
 
         return loader;