JDK 9 RFR of JDK-8035452: Fix serial lint warnings in core libs (original) (raw)

Stuart Marks stuart.marks at oracle.com
Wed Mar 5 01:41:46 UTC 2014

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

More information about the core-libs-dev mailing list