[rfc][icedtea-web] Fix for PR1251: AppContext has wrong context classloader

Fri Apr 5 09:18:15 PDT 2013

On 01/15/2013 05:53 PM, Adam Domurad wrote:
> This fixes one of the (numerous) issues in ADOMII, and fixes SweetHome3D furniture dragging.
>
> I'll preface by saying, I'm not particularly fond of the changes in this patch, but they have to be done to fix JNLP applications that load resources on the Swing thread, like eg SweetHome3D.
>
> The patch adds a flag to JNLPClassLoader that specifies whether it has been initialized. The default for getInstance remains the same, it grabs an initialized JNLPClassLoader. However an alternate getInstance was added to be able to grab JNLPClassLoader that hasn't been fully initialized.
>
> The problem with initialization:
> - To create AppContext we need JNLPClassLoader object constructed
> - To construct JNLPClassLoader we must initialize it
> - Initializing JNLPClassLoader implies downloading all resources, and updating Swing components
>
> However, we cannot update swing components before we create our AppContext, or the result will be a mess (eg unclickable popups).
> In essence, it's a bootstrapping problem. So the patch makes it possible to get a constructed JNLPClassLoader, which is initialized in a separate step.
>
> What else was tried ?
> The only other option is to set the context classloader to some other classloader. A proxy classloader can be created that initially points to the system classloader, but can be set to JNLPClassLoader after it is created. Determined too much of a hack, and getParent cannot be properly proxied.
>
> Basically with this patch, the life-time of the context classloaders for each thread is:
> 1. By default system classloader is context classloader.
> 2. Create new AppContext, capture JNLPClassLoader that isn't yet valid. Ensure we keep using system classloader as context classloader for all threads.
> 3. Initialize JNLPClassLoader. Before start of applet, ensure all threads context classloaders are set to (now initialized) JNLPClassLoader.
>
> Recommendations welcome -- it was a lot of deliberation before I decided to take this route. If someone has a simpler/cleaner solution than by all means suggest it!
>
> Reproducer changelog:
> 2013-XX-XX Adam Domurad <adomurad at redhat.com>
>
> * tests/reproducers/signed/AppContextHasJNLPClassLoader/resources/AppContextHasJNLPClassLoader.html:
> Test AppContext context classloader from HTML applet
> * tests/reproducers/signed/AppContextHasJNLPClassLoader/resources/AppContextHasJNLPClassLoader.jnlp:
> Test AppContext context classloader from JNLP application
> * tests/reproducers/signed/AppContextHasJNLPClassLoader/resources/AppContextHasJNLPClassLoaderForJNLPApplet.jnlp:
> Test AppContext context classloader from JNLP applet
> * tests/reproducers/signed/AppContextHasJNLPClassLoader/srcs/AppContextHasJNLPClassLoader.java:
> Print out context classloader for thread & AppContext, for
> current thread & Swing thread.
> * tests/reproducers/signed/AppContextHasJNLPClassLoader/testcases/AppContextHasJNLPClassLoaderTest.java:
> Test runner for AppContextHasJNLPClassLoader
>
> REPRODUCER NOTES:
> This exposes an additional bug in JNLP applets. start/init incorrectly runs on the Swing thread, and invokeAndWait hangs forever. I'm not sure how this wasn't caught, but the JNLP applet reproducer is broken pending a separate bug fix.
>
> Fixes changelog:
> 2013-XX-XX Adam Domurad <adomurad at redhat.com>
>
> Fix PR1251: AppContext has wrong context classloader
> * netx/net/sourceforge/jnlp/Launcher.java:
> Remove unused context flag. Make new AppContext capture not-yet-
> initialized JNLPClassLoader.
> * netx/net/sourceforge/jnlp/NetxPanel.java:
> Move PluginBridge initialization into constructor.
> Make new AppContext capture not-yet-initialized JNLPClassLoader.
> * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java:
> Add initialization flag. Move part of constructor into initialize
> method. Modify getInstance to allow temporarily bypassing
> initialization.
> * netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java
> (createAppContext): New, creates an AppContext that captures the given
> (not necessarily yet valid)
>
> -Adam

The reproducer have to go in with your new patch.
It should go both to 1.4  and to head (with proper known to fail).
But needs fixing - missing @TestInBrowser annotation is quite crucial :)

Without patch, all three failed (but thebrowser test because of unset browser)

with patch (after fixing of annotation)
Passed: AppContextHasJNLPClassLoaderTest.testJNLPApplicationAppContext
FAILED: testJNLPAppletAppContext(AppContextHasJNLPClassLoaderTest) stdout should contains 'main-thread: thread context classloader == JNLPClassLoader', but did not
  - This test is known to fail
Passed: AppContextHasJNLPClassLoaderTest.testAppletAppContext - google-chrome

Should the jnlp fial? ( i guess yes as it is mark as known), but have faile dto early:

/*
Connecting  /home/jvanek/icedtea-web-image/bin/javaws http://localhost:56174/AppContextHasJNLPClassLoaderJNLPApplet.jnlp

Fri Apr 05 18:10:40 CEST 2013

net.sourceforge.jnlp.ServerAccess.executeProcessUponURL(ServerAccess.java:680)
net.sourceforge.jnlp.ServerAccess.executeJavaws(ServerAccess.java:589)
net.sourceforge.jnlp.ServerAccess.executeJavaws(ServerAccess.java:550)
AppContextHasJNLPClassLoaderTest.testJNLPAppletAppContext(AppContextHasJNLPClassLoaderTest.java:86)
sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
java.lang.reflect.Method.invoke(Method.java:616)
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
net.sourceforge.jnlp.browsertesting.BrowserTestRunner.runLeaf(BrowserTestRunner.java:162)
net.sourceforge.jnlp.browsertesting.BrowserTestRunner.runChildX(BrowserTestRunner.java:137)
net.sourceforge.jnlp.browsertesting.BrowserTestRunner.runChild(BrowserTestRunner.java:106)
net.sourceforge.jnlp.browsertesting.BrowserTestRunner.runChild(BrowserTestRunner.java:58)
org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
org.junit.runners.ParentRunner.run(ParentRunner.java:236)
org.junit.runners.Suite.runChild(Suite.java:128)
org.junit.runners.Suite.runChild(Suite.java:24)
org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
org.junit.runners.ParentRunner.run(ParentRunner.java:236)
org.junit.runner.JUnitCore.run(JUnitCore.java:157)
org.junit.runner.JUnitCore.run(JUnitCore.java:136)
org.junit.runner.JUnitCore.run(JUnitCore.java:117)
CommandLine.runMain(CommandLine.java:49)
CommandLine.runMainAndExit(CommandLine.java:28)
CommandLine.main(CommandLine.java:24)

and now the important one:  */

Attempted to download http://localhost:56174/AppContextHasJNLPClassLoaderJNLPApplet.jnlp, but failed to connect!
netx: Read Error: Could not read or parse the JNLP file. (http://localhost:56174/AppContextHasJNLPClassLoaderJNLPApplet.jnlp)

  reproducers_test_server_deploydir]$ ll AppContextHasJNLPClassLoader*
-rw-rw-r-- 1 jvanek jvanek 2457 Apr  5 18:10 AppContextHasJNLPClassLoaderForJNLPApplet.jnlp
-rw-rw-r-- 1 jvanek jvanek 1851 Apr  5 18:10 AppContextHasJNLPClassLoader.html
-rw-rw-r-- 1 jvanek jvanek 3238 Apr  5 18:08 AppContextHasJNLPClassLoader.jar
-rw-rw-r-- 1 jvanek jvanek 2355 Apr  5 18:10 AppContextHasJNLPClassLoader.jnlp

Wrong name perhaps?

J.

Anyway why was the review of initial patch[1] abandoned?
[1] [rfc][icedtea-web] Fix for PR1251: AppContext has wrong context	classloader - in january 2013