[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