[rfc][icedtea-web] A dead-lock / Firefox hanging fix

[Adam Domurad]

Hi all.

Recently on one of my computers I noticed that Firefox was hanging every 
time an applet failed. While I'm not sure why this was suddenly 
happening every time, I took it as an opportunity to look into a fix 
while the bug was in plain sight.

The problem area is here:

plugin/icedteanp/java/sun/applet/PluginAppletViewer.java:740 (from HEAD)
>                 while (panel == null || !panel.isAlive()) {
>                                          ^^^^^^^^^^^^^
>                     maxTimeToSleep -= waitTillTimeout(panelLock, 
> panelLive,
> maxTimeToSleep);
>
>                     /* we already waited till timeout, give up here 
> directly,
>                      *  instead of waiting 180s again in below 
> waitForAppletInit()
>                      */
>                     if(maxTimeToSleep < 0) {
>                         streamhandler.write("instance " + identifier + 
> " reference " + -1 + " fatalError: " + "Initialization timed out");
>                         return;

The intent for using panel.isAlive() in the condition here is presumably 
to ensure that the handler thread is created. NetxPanel is created with 
appletAlive=true, so the following method:

netx/net/sourceforge/jnlp/NetxPanel.java:196 (from HEAD)
>     public boolean isAlive() {
>         return handler != null && handler.isAlive() && this.appletAlive;
>     }

Effectively tests that the handler thread was created. There's one 
problem, in the case of an exception path ...

netx/net/sourceforge/jnlp/NetxPanel.java:150 (from HEAD)
>         } catch (Exception e) {
>             this.appletAlive = false;
>                   ^^^^^^^^^^^^^^^^^^
>             e.printStackTrace();
> replaceSplash(SplashUtils.getErrorSplashScreen(getWidth(), 
> getHeight(), e));

This unfortunately causes isAlive() to permanently return false from 
that point onwards. Clearly if isAlive() ever gets false in the 
condition at PluginAppletViewer:740 we will loop forever. The C++ side 
of the plugin can do nothing but wait. Additionally, since icedtea-web 
is run in-process unlike other plugins, this hangs firefox completely.

One thing to notice is, PluginAppletViewer:740-751 are fairly redundant 
because line 760 (== 'waitForAppletInit(panel)') waits for complete 
initialization anyway.
The patch therefore axes these lines, which were mostly identical to 
waitForAppletInit. There is no need to separately wait for the handler 
thread to initialize. It's also worth noting that waiting on panel == 
null makes little sense because 'panel' is set *only* in the 
constructor. Therefore if it were ever null we would be spinning forever 
anyway.

The other part of the fix was making sure the C++ side of the plugin 
gets a response.

plugin/icedteanp/java/sun/applet/PluginAppletViewer.java:764 (from HEAD)
>             if (panel.getApplet() == null) {
>                 streamhandler.write("instance " + identifier + " 
> reference " + -1 + " fatalError: " + "Initialization failed");
>                 return;
>             }

This lacks a response to the message being handled, meaning the C++ side 
will wait until timeout. Adding 'streamhandler.write("context 0 
reference " + reference + " Error");' solves the problem neatly.


ChangeLog:
2013-XX-XX  Adam Domurad  <adomurad at redhat.com>

         Fix a dead-lock that can cause (namely) Firefox to hang.
         * netx/net/sourceforge/jnlp/NetxPanel.java
         (appletAlive): Remove flag.
         (isAlive): Remove getter.
         (initialized): New, explicit initialization flag.
         (isInitialized): New, getter.
         (runLoader): Set initialization flag when done (whether errored 
or not).
         * plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
         (waitForAppletInit): Wait on initialization flag from NetxPanel.
         (handleMessage): Remove redundant waiting for init. Respond 
properly to
         GetJavaObject in case of error/time-out.


Happy hacking,
-Adam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: NetxPanel-init-deadlock.patch
Type: text/x-patch
Size: 5005 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130416/0a13b8cc/NetxPanel-init-deadlock.patch