[icedtea-web[ broken elluminate on head

[Andrew Azores]

I've been doing some more testing of the patch(es) for Elluminate and multi-applet pages on a couple of different systems and so far I haven't had a single problem, so I'm feeling like perhaps this is already sufficient. You're right that 1.4 head and 1.4.1 releases both don't appear to be affected, but I believe the problem is still probably there, just isn't quite being hit, since this does seem to be a timing problem.

I know we were discussing this on IRC a few days ago but just to clarify again - ok to push? And do you think it should go into 1.4 as well now or only head?

----- Original Message -----
From: "Jiri Vanek" <jvanek at redhat.com>
To: "Andrew Azores" <aazores at redhat.com>, "IcedTea Distro List" <distro-pkg-dev at openjdk.java.net>
Sent: Wednesday, December 18, 2013 3:05:42 AM
Subject: Re: [icedtea-web[ broken elluminate on head

On 12/17/2013 08:53 PM, Andrew Azores wrote:
> 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.
>

Wait - 1.4 (head of 1.4) is *not* affected. So core issue have to be somwhere else. Maybe your patch 
just made it visible.



J.