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

[Joe Darcy]

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
>
>
>
>