Hi Brent,
On 05/04/2015 09:11 AM, 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/
Thanks for taking this on. I haven't seen an updated webrev yet and I
reviewed webrev.0 version.
Peter is right that the CheckOverrides.java test doesn't need to match
the checked exceptions in the method signature to determine if a new
method is missing in Properties class to override. If any checked
exception is changed in a public/protected Hashtable method,
Properties.java should fail the compilation.
W.r.t. whether Properties.load* methods should keep the synchronized,
the existing behavior of Properties.load* methods update the Properties
object with all entries before any other thread calls other write
operation (e.g. setProperty or put). As David pointed out, this patch
changes the current behavior that the resulting Properties after the
load method call might be different if concurrent modification is done
by a separate thread. I'm unsure if this incompatibility is a real
concern in practice due to the nature concurrency. In order to have
predictable result, an application should have their own lock to ensure
the Properties entries are updated in the expected order as far as I can
see. On the other hand, I'd like Daniel's suggestion to have the load*
method to putAll entries in one go after the input reader/stream/xml
file is loaded and parsed rather than adding one entry at a time.
Taking another look at this deadlock issue and the compatibility
concerns, I wonder if we should keep this change as a special
implementation for system properties rather than having this change to
java.util.Properties class. Properties is a Hashtable which specifies
the fast-fail behavior (throwing ConcurrentModificationException for
concurrent update). There are other issues specific to system
properties we want to clean up (e.g. read-only system property, private
system property to JDK but not visible to public etc).
Any thought?
Mandy
