JDK 9 RFR of JDK-8035452: Fix serial lint warnings in core libs

[Stuart Marks]

Hi Joe,

These changes are fine. You might want to update the copyright year of 
ExceptionProxy.java though.

s'marks

On 3/3/14 10:05 PM, Joe Darcy wrote:
> Hi Stuart,
>
> Thanks for the careful review. I agree the case of TreeMap.NavigableSbugMap
> requires some more investigation. In the meantime, I'd like to proceed with the
> revised patch below for EnumSet and ExceptionProxy.
>
> Thanks,
>
> -Joe
>
> diff -r 6cfedc362f48 src/share/classes/java/util/EnumSet.java
> --- a/src/share/classes/java/util/EnumSet.java    Mon Mar 03 18:17:00 2014 +0400
> +++ b/src/share/classes/java/util/EnumSet.java    Mon Mar 03 22:02:06 2014 -0800
> @@ -1,5 +1,5 @@
>   /*
> - * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights reserved.
>    * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>    *
>    * This code is free software; you can redistribute it and/or modify it
> @@ -77,6 +77,8 @@
>    * @see EnumMap
>    * @serial exclude
>    */
> + at SuppressWarnings("serial") // No serialVersionUID due to usage of
> +                            // serial proxy pattern
>   public abstract class EnumSet<E extends Enum<E>> extends AbstractSet<E>
>       implements Cloneable, java.io.Serializable
>   {
> diff -r 6cfedc362f48 src/share/classes/sun/reflect/annotation/ExceptionProxy.java
> --- a/src/share/classes/sun/reflect/annotation/ExceptionProxy.java Mon Mar 03
> 18:17:00 2014 +0400
> +++ b/src/share/classes/sun/reflect/annotation/ExceptionProxy.java Mon Mar 03
> 22:02:06 2014 -0800
> @@ -37,5 +37,6 @@
>    * @since   1.5
>    */
>   public abstract class ExceptionProxy implements java.io.Serializable {
> +    private static final long serialVersionUID = 7241930048386631401L;
>       protected abstract RuntimeException generateException();
>   }
>
>
> On 02/28/2014 08:58 AM, Stuart Marks wrote:
>> On 2/27/14 12:11 PM, Joe Darcy wrote:
>>> I am trying hard to remain blissfully ignorant of any more low-level details of
>>> the serialization format; however, I might not be successful on that goal much
>>> longer ;-)
>>
>> I believe your latter statement is correct. :-)
>>
>>> My preference in a case like this is to add the svuid if for no other reason
>>> that is is simple to explain and understand, even if it is not strictly
>>> required.
>>
>> In general, it does seem reasonable to add a svuid in cases where it's
>> difficult or impractical to prove that the lack of a svuid causes no
>> compatibility issues.
>>
>>> There is a difference in character between a serializable class in Java SE
>>> (java.* and javax.*) and the jdk.Exported(true) types in the JDK and a
>>> serializable class that lives in sun.* or some other jdk.Exported(false) area.
>>>
>>> For that latter, the serialization contract has to be different, with fewer
>>> guarantees, just as the general usage contract for those types has fewer
>>> guarantees. I think this is analogous to putting non-serializable classes into
>>> collections; the collection itself is serializable, but it won't be anymore if
>>> you put non-serializable objects into it.
>>>
>>> If a user happens to have a direct or indirect reference to an object of a JDK
>>> implementation type, the compatibility contract is weaker than if an object with
>>> a public Java SE type were being dealt with.
>>
>> I'm not sure I buy this. Unfortunately, serialization differs from the general
>> usage contract in that serialization exposes internals. Just like it can
>> expose private (non-transient) fields, serialization can also can expose what
>> might otherwise look like purely internal implementation types.
>>
>> The canonical example of how an application can get a reference to an
>> apparently internal class is java.util.TimeZone. If an app calls
>> TimeZone.getDefault(), it gets back an instance of sun.util.calendar.ZoneInfo
>> without ever mentioning this class by name. Furthermore, TimeZone and ZoneInfo
>> are serializable, so they can be serialized directly or as part of another
>> serializable object graph, and an instance of s.u.c.ZoneInfo does appear in
>> the serialized byte stream. If s.u.c.ZoneInfo were not to have a svuid, an
>> apparently innocuous change to it would clearly cause application
>> incompatibilities.
>>
>> As it happens, s.u.c.ZoneInfo does have a svuid. I also note in passing that
>> TimeZone, an abstract class, also has a svuid.
>>
>>>> Finally, EnumSet doesn't need a serial version UID. It's serialized using a
>>>> proxy class, so EnumSet never appears in a serialized byte stream. (Note, its
>>>> readObject throws an exception unconditionally.) So it's probably safe to
>>>> suppress its serialization warning.
>>>
>>> Yes, EnumSet was a bit tricky, it is serializable itself, but uses a proxy
>>> internally. ("Effective Java, 2nd edition" both recommends the proxy pattern and
>>> recommends adding a svuid to all serializable classes, but doesn't explicitly
>>> give guidance to this combination of features.)
>>>
>>> To avoid adding a long comment explaining the proxy pattern and why a svuid on
>>> EnumSet isn't really required, my preference would just be to add the svuid if
>>> it doesn't cause any harm.
>>
>> In this case I think it's possible by local reasoning (looking elsewhere in
>> the EnumSet.java source code) to determine that EnumSet instances themselves
>> are never actually serialized, because of the use of the serialization proxy
>> pattern. If nothing else, adding a svuid here sets a bad example, so
>> suppressing the warning is reasonable. I don't think a long comment is
>> necessary. Probably something like the following is sufficient:
>>
>> @SuppressWarnings("serial") // no serialVersionUID because of serialization proxy
>>
>> s'marks
>>
>>
>>
>>
>