[rfc][icedtea-web] function applet -> js ->applet call deadlock

[Adam Domurad]

On 05/15/2013 07:56 AM, Jiri Vanek wrote:
> When applet calls JavaScript function, which calls back to appelt, 
> then *mostly* deadlock occurs. I made several attempts to fix this 
> more generally, but I was unsuccessful.
>
> So this "hack" is preventing deadlock, maybe also the timeout can be a 
> bit shorter...
>
> Although this is for both head and  1.4, for head some more 
> investigations are needed later.
>
> J.
>
> ps,reproducer in progress.

Hi, thanks for looking into it. Probably good idea to remove dead-lock 
potential.

> diff -r 9f2d8381f5f1 plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java	Fri May 03 16:17:08 2013 +0200
> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java	Mon May 06 16:06:59 2013 +0200
> @@ -1305,8 +1305,16 @@
>               PluginDebug.debug("wait ToString request 1");
>               synchronized (request) {
>                   PluginDebug.debug("wait ToString request 2");
> -                while (request.isDone() == false)
> -                    request.wait();
> +                int counter = 0;
> +                while (request.isDone() == false){
> +                    // Do not wait indefinitely to avoid the potential of deadlock
> +                    // but this will destroy the  intentional recursion ?
> +                    counter++;
> +                    if (counter>10){
> +                        throw new InterruptedException("Possible deadlock, releasing");
> +                    }
> +                    request.wait(1000);
> +                }
>                   PluginDebug.debug("wait ToString request 3");
>               }
>           } catch (InterruptedException e) {
>

This is more complex than it needs to be.
More simple is:

if (!request.isDone()) {
     request.wait(REQUEST_TIMEOUT);
}
if (!request.isDone()) {
   // Do not wait indefinitely to avoid the potential of deadlock
   throw new RuntimeException("Possible deadlock, releasing");
}

Your message gets tossed aside and a RuntimeException is thrown if you 
throw InterruptedException, more direct is better.

Also please put this in its own method, eg waitForRequestCompletion 
(probably good to encapsulate the 'catch InterruptedException' here). 
There are many methods like this that have a wait loop.
Though I would like to see the reproducer before commenting more.

Thanks,
-Adam