Icedtea-web splashscreen implementation

Omair Majid omajid at
Thu Nov 1 09:30:24 PDT 2012

On 11/01/2012 11:32 AM, Jiri Vanek wrote:
> On 10/29/2012 07:42 PM, Omair Majid wrote:
>> Hi Jiri,
>> Comments in-line below.
> Thank you :)
> But you are giving me hard times. It needs a lot of garbage-searching in
> deep corners of my nearly-forgotten memories to recall why I have done
> something.

Err... sorry.

>>>   }
>>> diff -r c52fcd37dbd8 netx/net/sourceforge/jnlp/
>>> --- a/netx/net/sourceforge/jnlp/    Tue Oct 23 09:56:26
>>> 2012 +0200
>>> +++ b/netx/net/sourceforge/jnlp/    Wed Oct 24 11:46:08
>>> 2012 +0200
>>>       protected ApplicationInstance launchApplet(JNLPFile file,
>>> boolean enableCodeBase, Container cont) throws LaunchException {
>>> +        if (JNLPRuntime.getForksAllowed()&&  file.needsNewVM()) {
>>> +            if (!JNLPRuntime.isHeadless()) {
>>> +                SplashScreen sp = SplashScreen.getSplashScreen();
>>> +                if (sp != null) {
>>> +                    sp.close();
>>> +                }
>>> +            }
>> This is really puzzling. Forks are never allowed for applets, so the
>> first if statement should never evaluate to true. But if we could start
>> new VMs for applets, we would need to prevent applets from running in
>> this VM. We need to return from this method to avoid executing further
>> code, but we also need to notify the plugin that another VM now handles
>> the plugin.
> Really? Not even when launched from jnlfile even with xmx?? I thin there
> is no reason  not to allow them...
> However I have followed your advice. But I really think that applets
> launched via jnlp should be able to fork...

Ah, you are right, of course! Can you post this particular change as a
separate patch? It is not related to the rest of the patch and I will
okay it ASAP.

But you still need to create a new JVM and make it run that applet and
stop this JVM from running it.

>>> -    protected AppletInstance createApplet(JNLPFile file, boolean
>>> enableCodeBase, Container cont) throws LaunchException {
>>> +    protected  synchronized  AppletInstance createApplet(JNLPFile
>>> file, boolean enableCodeBase, Container cont) throws LaunchException {
>> Okay, this "synchronized" keyword here looks rather out of place. If
>> synchronization is really needed, I don't think we should be adding it
>> to this class.
> Well here we have an disagreeing. I have played with those two newly
> synchronised methods again.
> When two (or even more) applets are on same page, launched together,
> without both method synchronised the splashscrens are initiated wiredly:(
> After those tests I have kept those new synchronisation in code.
> What is actually your reason not to add them?

As far as I can tell there are no existing 'synchronized' methods in
this class. I am not sure if this class is meant to be thread safe or
not. It forks a bunch of threads and then waits for them to join but I
don't see any synchronization otherwise. This addition of synchronized
adds additional responsibility to the class, but without any other
details or explanation.

I would be okay with adding them if you could clarify why they are
needed here. Instead of adding 'synchronized' to two methods, lets find
out why that weirdness is happening and the best way to fix it. Does
this class need to be made thread-safe (and the documentation for this
class updated)? Or do we need to use some higher level primitives to
ensure exclusivity? Unless we understand the actual problem, this fix
might turn out to be a bad decision. It may even cause deadlocks if we
don't fully understand its consequences.

If you really think this is needed right now, then please add a FIXME at
least so we can double check this later. But I would really prefer to
understand and fix the underlying problem correctly once and for all.

I am okay with everything else in the patch.


PGP Key: 66484681 (
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681

More information about the distro-pkg-dev mailing list