[rfc][icedtea-web] Fix for PR1251: AppContext has wrong context classloader
Adam Domurad
adomurad at redhat.com
Thu Apr 18 10:44:08 PDT 2013
On 04/08/2013 09:17 AM, Jiri Vanek wrote:
> On 04/05/2013 08:08 PM, Adam Domurad wrote:
>> 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 AppConte
>
> xtHasJNLPClassLoader.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.
>>
>
> Ach, one more improvemnt - sorry for late notice. Can you add closing
> listeners for browser tests for case of success?
It already has one, unless I misunderstand you. (It has an auto-closer,
anyway.)
> With it it can go to 1.4 with your sweet home patch. But for head
> should be all methods as knonwToFial. You can add link to @Bug to the
> 1.4 hack push to keep line between head and 1.4.
>
> Please send to one more round both for 1.4 and head when time will come.
>
> J
I'll add @KnownToFail before I push.
Happy hacking,
-Adam
More information about the distro-pkg-dev
mailing list