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

Fri Apr 5 11:08:21 PDT 2013

On 04/05/2013 12:18 PM, Jiri Vanek wrote:
> 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

Fixed

> Passed: AppContextHasJNLPClassLoaderTest.testAppletAppContext - 
> google-chrome
>
>
> Should the jnlp fial? ( i guess yes as it is mark as known), but have 
> faile dto early:

See http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1253 for why 
it is known-to-fail

>
> /*
> 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?

Fixed

>
> 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

I spent a while on it struggling with the correct design before I 
started working on other things. I still intend to get back to it, but 
the initial patch & approach I worked on was rejected. I still need to 
come up with a good way to approach this; it isn't trivial.

Happy hacking,
-Adam

-------------- next part --------------
A non-text attachment was scrubbed...
Name: appcontext-reproducer2.patch
Type: text/x-patch
Size: 16570 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130405/cc2f550f/appcontext-reproducer2.patch 