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

[Jiri Vanek]

Here you go:)


J.

On 05/15/2013 05:55 PM, Adam Domurad wrote:
> 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
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: dadlockFix-tests.patch
Type: text/x-patch
Size: 9807 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130517/64612323/dadlockFix-tests.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dadlockFix-fix.patch
Type: text/x-patch
Size: 1334 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130517/64612323/dadlockFix-fix.patch