On Sun, 8 Nov 2020 15:55:58 GMT, Peter Levart <plev...@openjdk.org> wrote:
>> Hi Stuart,
>>
>> I would like to discuss the serialization. You introduce new
>> CollSer.IMM_LIST_NULLS type of immutable collection. This means that if this
>> change goes into JDK16 for example, JDK15 and before will not be able to
>> deserialize such list as they know nothing about IMM_LIST_NULLS even if such
>> lists don't contain nulls. The reason you say to chose new type of
>> serialization format is the following:
>>
>>> "Suppose I had an application that created a data structure that used lists
>>> from List.of(), and I had a global assertion over that structure that it
>>> contained no nulls. Further suppose that I serialized and deserizalized
>>> this structure. I'd want that assertion to be preserved after
>>> deserialization. If another app (or a future version of this app) created
>>> the structure using Stream.to
>>> List(), this would allow nulls to leak into that structure and violate
>>> that assertion. Therefore, the result of Stream.toList() should not be
>>> serialization-compatible with List.of() et. al. That's why there's the new
>>> IMM_LIST_NULLS tag in the serial format"
>>
>> I don't quite get this reasoning. Let's try to decompose the reasoning
>> giving an example. Suppose we had the following data structure:
>>
>> public class Names implements Serializable {
>> private final List<String> names;
>> Names(List<String> names) {
>> this.names = names;
>> }
>> public List<String> names() { return names; }
>> }
>>
>> App v1 creates such structures using new Names(List.of(...)) and
>> serializes/deserializes them. They keep the invariant that no nulls are
>> present. Now comes App v2 that starts using new Names(stream.toList()) which
>> allows nulls to be present. When such Names instance from app v2 is
>> serialized and then deserialized in app v1, nulls "leak" into data structure
>> of app v1 that does not expect them.
>>
>> But the question is how does having a separate CollSer.IMM_LIST_NULLS type
>> prevent that from happening?
>
> I can see that having a separate IMM_LIST_NULLS type might be necessary to
> preserve the allows-null/disallows-null behaviour of indexOf and lastIndexOf
> methods...
>
> NOTE ALSO that ListN.equals(o) and ListN.hashCode() are inherited from
> AbstractImmutableList:
>
> @Override
> public boolean equals(Object o) {
> if (o == this) {
> return true;
> }
>
> if (!(o instanceof List)) {
> return false;
> }
>
> Iterator<?> oit = ((List<?>) o).iterator();
> for (int i = 0, s = size(); i < s; i++) {
> if (!oit.hasNext() || !get(i).equals(oit.next())) {
> return false;
> }
> }
> return !oit.hasNext();
> }
> and
> public int hashCode() {
> int hash = 1;
> for (int i = 0, s = size(); i < s; i++) {
> hash = 31 * hash + get(i).hashCode();
> }
> return hash;
> }
>
> ...which means they will throw NPE when the list contains null. The same goes
> for SubList.
@plevart wrote:
> But the question is how does having a separate CollSer.IMM_LIST_NULLS type
> prevent that from happening?
When a serialized list with IMM_LIST_NULLS is deserialized on an older JDK,
it'll throw InvalidObjectException since that tag isn't valid on older JDKs.
Obviously this is still an error, but it's a fail-fast approach that avoids
letting nulls leak into a data structure where they might cause a problem some
arbitrary time later.
> NOTE ALSO that ListN.equals(o) and ListN.hashCode() are inherited from
> AbstractImmutableList [...] which means they will throw NPE when the list
> contains null. The same goes for SubList.
Good catch! Yes, this is a problem. I'll do some rearranging here and add more
test cases. Thanks for spotting this.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1026
