RFR: 8231640: (prop) Canonical property storage [v17]

[Jaikiran Pai]

On Wed, 15 Sep 2021 16:13:56 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> src/java.base/share/classes/java/util/Properties.java line 932:
>> 
>>> 930:             if (entries instanceof Collections.SynchronizedSet<?> ss
>>> 931:                     && ss.c instanceof EntrySet) {
>>> 932:                 entries = new ArrayList<>(entries);
>> 
>> From the description referring to a 'different instance', I expected to see == or != in this test.
>> Since Properties.entrySet() always returns a new synchronized set of a new EntrySet,
>> specifying that the underlying map is the same instance is more difficult.
>> Perhaps either the SynchronizedSet or the EntrySet instance could be cached and compared.
>> 
>> (typo: missing space after period... ".If yes")
>
> A custom subclass couldn't create an instance of EntrySet directly. Therefore if it wants to reorder the entries, it would have to return a new TreeSet or LinkedHashSet - or some other custom set implementation. If the result of calling entrySet() is an EntrySet wrapped in an UnmodifiableSet it's therefore legitimate to assume that the set hasn't been reordered, and that we can reorder it. Or am I missing something?

I've updated the PR to clarify what this part of the code is doing. I took Daniel's input to reword the javadoc as well as tried to improve the code comment where this check is done. Essentially, we check for the "type" of the returned intstance and if we see that it's a `SynchronizedSet` whose underlying collection is a private class `EntrySet` then we know that it's the one created by Properties.entrySet(). Like Daniel says, subclasses won't be able to create or use this internal private class and if they did override the entrySet() to change the order of the entries they would have to do it by returning us a different "type" from that method.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5372