[rfc][icedtea-web] RH976833 ClassLoader deadlock

[Andrew Azores]

On 10/04/2013 03:42 PM, Omair Majid wrote:
>
>> As for the rest of your response... well, I'm going to rethink this
>> whole thing and try to come up with a better, higher-level solution to
>> this. It is scary dealing with low-level synchronization like this,
>> especially in somewhere like the JNLPClassLoader. Like I said, it's
>> possible to simply use a ReentrantLock or other similar construct and
>> retain the coarse-grained synch control over the
>> JNLPClassLoader.loadClass method, without resorting to low-level
>> synchronization on instance variables. In terms of the diagram this
>> would mean introducing a new lock "r1", and causing the second box on
>> the chart to cause the acquisition of r1 rather than cl1. As far as I
>> can tell there's nothing really wrong with enforcing that only one
>> thread enters that method at a time, the issue is just that the lock
>> used for this purpose happens to be an inappropriate lock to use in this
>> particular scenario.
> I await the patch :)
>
> Cheers,
> Omair
>

This should fix it. It's still a bit heavy-handed IMO, but on the other 
hand it avoids messy synchronization of individual fields. Reasoning 
about it should be much easier too, especially given that flow diagram. 
Like I said before, the JNLPClassLoader instance lock is not actually 
used anywhere else, except by inherited methods. So I think it's safe to 
do the "cl1/r1" replacement from my last message. AFAIK nothing else is 
depending on that lock being in use, really.

Just as correct (or incorrect) would have been to create a new Object 
and synchronize() on it instead, but it's a very long method and I 
didn't think it would look very good being indented another level.

Testing with the browserspy.dk website mentioned in the report as well 
as with the 9x applets page I linked somewhere earlier in this message 
thread, the deadlock is resolved. I haven't come up with a reproducer 
yet but I just need to figure out how to ensure that the reproducer 
causes the two applet threads to go through the same two classloaders as 
they are in that diagram. I'm not actually sure what the conditions are 
for that so I'll have to look into it unless anyone can explain it off 
the top of their head.

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: reentrant_lock.patch
Type: text/x-patch
Size: 1184 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20131004/6e609adf/reentrant_lock.patch