[rfc][icedtea-web] PR1251: Hack for appcontext & event-queue classloaders

[Adam Domurad]

On 04/05/2013 10:02 AM, Jiri Vanek wrote:
> On 04/04/2013 10:17 PM, Adam Domurad wrote:
>> This puts a bandaid over 
>> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1251. 'Fix' is 
>> probably too generous.
>> With this, the SweetHome3D JNLP [1] runs again. The applet version 
>> had worked before.
>>
>> This is explicitly *not* wanted for HEAD. I am posting this for 
>> review now to be considered for any release branches where a proper 
>> fix is not found. (1.4 definitely, who knows maybe even previous & 
>> future releases. But such hackery should not be in HEAD.)
>>
>> 2013-XX-XX Adam Domurad <adomurad at redhat.com>
>>
>> Hack to ensure AppContext is set correctly.
>> * netx/net/sourceforge/jnlp/Launcher.java
>> (forceContextClassLoaderHack): Force stored class-loader for
>> thread-group's AppContext and EventQueue to be our JNLPClassLoader.
>> (createApplet): Call forceContextClassLoaderHack.
>> (createAppletObject): Call forceContextClassLoaderHack
>> (createApplication): Call forceContextClassLoaderHack.
>>
>> I have not found any potential problems this may cause (and I've 
>> digged quite deep). Let me know of any concerns.
>>
>> [1] www.sweethome3d.com/SweetHome3D.jnlp
>>
>> Happy hacking,
>> -Adam
>
> Hi! Thanx for this.
>
> Can you write (point to) more information about the (possible) patch 
> for head?
>
> Anyway I agree this is hack. When 1.4 is branched, I'm ok for this to 
> push to it. Anyway It will need some focus during 1.4 release 
> regression testing  to be sure we have not broken anything....
> Btw - changes like this NEEDS reproducers.  The rule from wiki[1] is 
> not obeyed those times too offten. The development of ITW have speed 
> up a lot now, but we are loosing a testcoverage a bit :(

While I certainly can't say I disagree, I do think some of the changes 
you are considering here apply broadly enough to already be covered by 
the reproducer suite (eg initialization changes). Also, I do sometimes 
write pure testing patches :-)

>
> [1] http://icedtea.classpath.org/wiki/CommitPolicy#OpenJDK_Patches
> "IcedTea-Web code changes/new feature should be accompanied with 
> appropriate tests (JUnit class and/or reproducer). If no tests are 
> added/modified, changes should be accompanied with an explanation as 
> to why. "
>
> J.

Sorry for not pointing to the reproducer (I actually thought this 
reproducer was already in HEAD).

Please check the email:
[rfc][icedtea-web] Fix for PR1251: AppContext has wrong context classloader
For the reproducer and additional information.

To summarize: JNLPClassLoader needs to be created before the AppContext. 
The naive approach results in unclickable pop-ups (although it's a 
little hard to explain why without digging into it again). The correct 
approach is to ensure all pop-ups occur on some 'security thread', or to 
have JNLPClassLoader creation be far more light-weight.

Sorry for fairly unsatisfying reply, I have lost the details a bit. I'll 
have to pick this up again some time.

Happy hacking,
-Adam