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

Reply via email to