[icedtea-web[ broken elluminate on head

[Jiri Vanek]

On 12/23/2013 03:33 PM, Andrew Azores wrote:
> 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?

Yes please, go on and push both to head and 1.4.
Please hold head push to tomorrow morning, so my automated test run will pick up head *without* your commit ( I wont to have clear run with new console and config singleton ). I had to reschedule the run because of firefox click and play :-/.
I will start another run 25th morning *with* your push and see the results.

Thanx for double checking and great investigations!


J.
>
> ----- 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.