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

[Stuart Marks]

On 2/24/14 8:22 PM, Joe Darcy wrote:
> On 02/20/2014 12:49 PM, Paul Benedict wrote:
>> Joe, I find it interesting that you suppressed the serial warning on an
>> abstract class. I'd like to know more about that. Is this a universal rule?
>>  Are serial uids not important for abstract classes?
>
> I wouldn't generalize that way from this example.
>
> The serial hash of NavigableSubMap has differed in different JDK releases,
> but its subclasses do define serialVersionUID fields. I assume the set of
> non-transient fields in NavigableSubMap has stayed unchanged, but I haven't
> verified that.
>
> If I hadn't found the change in the hash value, I would have added the
> serialVersionUID to NavigableSubMap too.

And in his reply to Paul, Joe said:
> From what I was able to discern by reading the serialization specification
> [1], If a class does *not* declare a serialVersionUID, the serialVersionUID
> of its superclass is *not* included in the serialver hash computation of the
> child class. However, my understanding is that changes to the fields stored
> in superclass and changes to the semantics of its readObject / writeObjects
> methods could affect the serialization of the child class.

I think we need to take a closer look at these issues.

I believe that abstract, serializable superclasses *do* need to have a 
serialVersionUID defined. The reason is that when a subclass is serialized, its 
superclass descriptor (an ObjectStreamClass) is also serialized. Upon 
deserialization, the descriptor's svuid is matched against the svuid of the 
class loaded at runtime, and if there is a mismatch, InvalidClassException ensues.

While the svuid of an abstract superclass isn't included in the subclass' svuid 
hash, the svuid of the superclass does affect serial compatibility of subclasses 
as described above. Thus, an apparently innocuous change to the superclass might 
prevent serial compatibility of its subclasses, no matter how carefully the 
subclasses are programmed.

If the NavigableSubMap class has changed svuid values over several releases, 
well, unfortunately we may have a compatibility problem already in the field. 
We'd need to choose which release to be compatible with. Since 8 isn't quite out 
yet, we might be able to change an early 8-update and 9 to be compatibile with 
the latest 7-update.

Note that the svuid of a class does not relate solely to the fields that are 
serialized. It's an attempt at a version hash of the *implementation* of a 
class, not a version of the serial protocol. Even changes to a class that don't 
affect the serialized output stream can affect the svuid. For example, renaming 
a package-private method will affect the svuid. See section 4.6 of the 
serialization spec.

While we're at it (sorry...) in the diffs for your other serial warnings patch 
JDK-8035453, there are several lines where the serial warning is suppressed like so:

+ at SuppressWarnings("serial") // JDK implementation class

As you know, serialization can expose the private fields of a class, making them 
public in a sense. Serialization can also expose what are internal, 
implementation classes, if these classes are part of a serializable object graph 
that is exposed to applications. I don't know about the specific situation with 
the DOM classes, but even if a serializable class is internal, we might need to 
be concerned about serialization compatibility.

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.

s'marks