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

Adam Domurad adomurad at redhat.com
Fri Jan 18 06:30:39 PST 2013


On 01/17/2013 06:15 PM, Omair Majid wrote:
> Hi Adam,
>
> I haven't looked into this in too much detail, but here are some quick
> thoughts.
>
> On 01/15/2013 11:53 AM, Adam Domurad wrote:
>> This fixes one of the (numerous) issues in ADOMII, and fixes SweetHome3D
>> furniture dragging.
> Nice! I can imagine that this must have been a pain to track down.

The key was setting a conditional breakpoint on 
Thread#setContextClassLoader that caught when the classloader went from 
JNLPClassLoader->system classloader. Having JRE source code is so useful!

>
>> 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.
> So far, this sounds just fine to me. Well, maybe we could avoid
> maintaining a record of ClassLoader instances, but that's a whole
> another problem.

We should move away from this with any theoretical reworking probably. 
(Working on this patch definitely made me itch to make JNLPClassLoader 
less heavy-weight, I wrote a rough list of all its responsibilities and 
its quite long.)

>
>> The problem with initialization:
>>      - To create AppContext we need JNLPClassLoader object constructed
>>      - To construct JNLPClassLoader we must initialize it
> Sounds perfectly fine so far.
>
>>      - Initializing JNLPClassLoader implies downloading all resources,
>> and updating Swing components
> What swing components do you imagine are being updated here?

Download indicator mainly + dialogues like "do you want to run this applet?"

>
>> 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.
> I am not a fan of the hack in JNLPRuntime.createNewAppContext where you
> set the system CL as the CCL on the EDT.
>
> We always have a thread (the security thread, I think it's called) that
> is running in the system appcontext that shows security prompts (see the
> SecurityDialogs class). Maybe you could add another thread (or extend
> the existing therad) to display icedtea-web UI (like splash screens and
> so on)?

Hack indeed so any alternatives appreciated. Can you give me some 
pseudo-code of what you're proposing? It would be greatly appreciated. 
I'm not sure what you mean display a UI in a separate thread -- AFAIK 
the only way to do this in a Swing-compatible way is to add components 
which will be drawn in the (current AppContext's) event-dispatch thread.

>
>>   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.
>>
> Nice catch!
>
> Thanks,
> Omair
>

-Adam



More information about the distro-pkg-dev mailing list