Code Review for 7001933: Deadlock in java.lang.classloader.getPackage()

[David Holmes]

HI Valerie,

Valerie (Yu-Ching) Peng said the following on 02/25/11 11:36:
> Can someone please review for 7001933: Deadlock in 
> java.lang.classloader.getPackage()?
> 
> http://cr.openjdk.java.net/~valeriep/7001933/webrev.00/
> 
> The fixes are straightforward - release the lock when delegating to the 
> parent.
> The change has been verified by the submitter.

1642                 synchronized (packages) {
1643                     if (packages.get(name) != null) {
1644                         packages.put(name, pkg);
1645                     }
1646                 }

This looks wrong to me. Shouldn't you only do the put if the get returns 
null? And if the get doesn't return null shouldn't the method return 
that result ie:

                 synchronized (packages) {
                     Package pkg2 = null;
                     if ((pkg2 = packages.get(name)) == null) {
                         packages.put(name, pkg);
                     }
                     else
                         pkg = pkg2;
                 }
                 ...
                 return pkg;

In general it is easy to break a deadlock by making an open-call, as you 
have done, but the harder question to answer is: what possible 
side-effects could there be due to races now that this method is not atomic?

Cheers,
David