[icedtea-web[ broken elluminate on head

[Andrew Azores]

On 12/16/2013 01:36 PM, Andrew Azores wrote:
> On 12/16/2013 09:53 AM, Andrew Azores wrote:
>> On 12/16/2013 08:14 AM, Jiri Vanek wrote:
>>> Afaik eluminate was  broken on head by
>>> http://icedtea.classpath.org/hg/icedtea-web/rev/744442d54cbf
>>>
>>> (changeset:   811:744442d54cbf
>>> user:        Andrew Azores <aazores at redhat.com>
>>> date:        Wed Oct 16 13:13:19 2013 -0400
>>> summary:     Resolve multiple-applet deadlock issue in JNLPClassLoade)
>>>
>>> Andrew, may you recheck?
>>>
>>> http://icedtea.classpath.org/wiki/IcedTea-Web-Tests#javaws
>>>
>>> J.
>>
>> Hmm, yes, seems to be having problems for me as well. Elluminate 
>> hangs while loading some extra asserts. Backing out that changeset 
>> does seem to resolve it. I'll look into what's happening here.
>>
>> Thanks,
>>
>
> I'm trying to investigate deeper to confirm that this is actually a 
> good fix, but...
>
> Remember back in the discussion thread for RH976833 (the bug that my 
> commit was targeted to fix), when I originally had that ugly, scary 
> patch? The one that wraps a bunch of the classloader's Collection 
> fields in Collections.synchronized* calls, and then added synchronized 
> blocks for them all over the place? Well, I went and changed the fix 
> to use that rather than the new "loadClassLock" Object solution that 
> we finally settled on - and the ugly, scary patch did the trick. 
> Elluminate works again, and the existing JNLPClassLoaderDeadlock 
> reproducer still passes (and so does the real-world test page that I 
> modelled it after).
>
> So as I said, I'll look into this deeper, because I haven't actually 
> managed to look at the state of the classloader when Elluminate 
> appears to deadlock, I just tried the other patch on a hunch. The old 
> patch didn't apply cleanly so I had to port it over, which will take 
> more work to make sure nothing important has changed in the meantime 
> as well. But, it should end up being more or less identical. I guess 
> in the meantime, find that RH976833 thread again and brush up on the 
> contents of the patch I'm talking about, because I think I might end 
> up having to put it up for re-review soon.
>
> Thanks,
>

Examining the Elluminate hang with a debugger revealed that the problem 
here is pretty much exactly the same locking issue that caused RH976833 
in the first place. The two attached patches are the same as what I 
initially proposed as the fix for RH976833, but rebased for the latest 
revisions on head and 1.4. The expected result of applying these patches 
is that the JNLPClassLoaderDeadlock reproducer will continue to pass, 
and Elluminate will now begin to successfully load.

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: RH976833-fix4-elluminate-regression.patch
Type: text/x-patch
Size: 13432 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20131217/ff0e97d0/RH976833-fix4-elluminate-regression-0001.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: RH976833-fix4-elluminate-regression-1.4.patch
Type: text/x-patch
Size: 13803 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20131217/ff0e97d0/RH976833-fix4-elluminate-regression-1.4-0001.patch