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
