RFR 8029891 : Deadlock detected in java/lang/ClassLoader/deadlock/GetResource.java

[Peter Levart]

On 05/04/2015 06:11 PM, Brent Christian wrote:
> Hi,
>
> Please review this fix, courtesy of Peter Levart (thanks!), that I 
> would like to get in.
>
> https://bugs.openjdk.java.net/browse/JDK-8029891
> http://cr.openjdk.java.net/~bchristi/8029891/webrev.0/
>
> There is some discussion of it in the bug report, starting at 2014-12-31.
>
> The problem, as stated by Mandy:
>
> "System Properties is a hashtable that synchronizes on itself for any 
> access. Currently System.getProperties returns the Properties instance 
> accessed by the system in which any application code might synchronize 
> on it (that's what the test is doing). The problem reported 
> JDK-6977738 is about Properties.store method that was fixed not to 
> synchronize on this instance. System property is a common way for 
> changing the default setting and so it's impractical to expect the 
> class loading code path not to call System.getProperty."
>
> This fix changes java.util.Properties to store its values in an 
> internal ConcurrentHashMap, ignoring its Hashtable heritage.  In this 
> way, Properties can be "de-sychronized": all methods inherited from 
> Hashtable are overridden, to remove synchronization, and delegate to 
> the internal CHM.
>
> The serialized form is unchanged.
>
>
> An alternative approach considered would be for System.getProperties() 
> to return a duplicate snapshot of the current Properties.  This 
> presents a compatibility risk to existing code that keeps a reference 
> to the return value of System.getProperties() and expects to either 
> read new properties added afterwards, or set properties on the cached 
> copy.
>
> -Brent
>

Hi Brent,

The test looks mostly good, but I would refrain from matching the 
exception types as part of method signature. Why?

- javac already performs all the necessary checks about declared 
exceptions if some method overrides another method
- overriding method can declare a subset and/or subtypes of checked 
exceptions of those than overridden method declares and this is OK.
- overriding/overridden methods can declare arbitrary unrelated sets of 
unchecked exceptions and this is OK.

Another question is whether return type should be considered part of the 
method signature. For JVM it is, but Java, the language, just matches 
the method's name and parameter types and javac makes sure that 
overriding method declarest the same return type or a subtype of 
overridden method's return type. Well, javac generates two methods in 
the later case - the one declared in the source code with covariant 
return type and a synthetic bridge method with overridden method's 
return type that just calls the declared method. 
Class.getDeclaredMethods() returns both methods in that case. So either 
"key" shape would work (one that takes returnType as part of key and one 
that doesn't).

I don't think you need or even want to check that Properties declares 
all the methods with signatures from all interfaces. Just checking 
superclass(es) would do, as Properties is not abstract and in order to 
compile it must implement all abstract methods (either by inheriting 
from superclass(es) or implementing them itself). Specifically, you 
would not want to enforce interface default methods to be overridden by 
Properties if they are not overridden by superclass(es) (as the 
interface default method can only call other methods in the interface or 
Object methods and you need not override such method in Properties for 
Properties to be OK).

Regards, Peter