EnumSet.class serialization broken - twice

[Stuart Marks]

Arrrrrggggh. Yet another serialization bug.

But yes, this is a bug. Thanks for finding and diagnosing it.

Like Martin, I've often forgotten that classes themselves can be included in a 
serial stream, as well as instances of those classes. In fact I seem to recall 
arguing that because EnumSet uses the serialization proxy pattern, instances of 
it should never appear in a legitimate serial stream. I think that's true. 
However, I sent on to say that because of this, there is no issue with 
serialization compatibility, and thus EnumSet didn't need a serialVersionUID. 
That's incorrect.

I'm uncomfortable with relaxing the serialization spec and mechanism to allow a 
class in the serial stream to have a different svuid from the one loaded in the 
running JVM. Offhand I don't know what problems it could cause, but it seems 
like a fundamental change that would lead to problems at some point.

Also, this is a problem with one class (so far...) and I don't think we should 
change the whole serialization mechanism to support it.

I'm thus leaning toward your first suggestion of adding a serialVersionUID 
declaration to EnumSet that matches the value from JDK 8. This would go into the 
current repo (JDK 14) and likely be backported to JDK 13.

It seems like a no-brainer to backport this to JDK 11 as well; this would 
provide broad compatibility across JDK 8 LTS, JDK 11 LTS, and current JDK 
releases. However, changing the svuid is a specification change. More 
investigation is necessary to figure out what would be involved in doing this.

Meanwhile, it would seem sensible to file a bug and start on a fix for the 
current release(s). Would you be able to do that?

Again, thanks for finding this.

s'marks

On 6/18/19 7:32 AM, Peter Levart wrote:
> 
> 
> On 6/18/19 4:00 PM, Martin Buchholz wrote:
>> Java Historian says:
>> I was a reviewer for Effective Java 3rd Edition and EnumSet is the canonical 
>> example of the Serialization Proxy pattern,
>> so I tried to make sure the pattern was implemented as perfectly as possible.
>> 8192935: Fix EnumSet's SerializationProxy javadoc
>> All of us who try to make java serialization work right have a mental model of 
>> the many things that might go wrong.
>> Serialization of Class objects has never been part of my own mental model - 
>> I've only ever considered instances.
> 
> Perhaps the necessity for Class objects representing Serializable classes to 
> agree in sertialVersionUID is a bug in Java serialization implementation? 
> There's no such requirement for Class objects representing non-Serializable 
> classes and I don't see why this requirement is there for Serializable classes. 
> Could this requirement simply be relaxed with no ill consequences?
> 
> Regards, Peter
> 
>>
>>
>> On Tue, Jun 18, 2019 at 5:32 AM Peter Levart <peter.levart at gmail.com 
>> <mailto:peter.levart at gmail.com>> wrote:
>>
>>     Hi,
>>
>>     I recently stumbled on an exception thrown when deserializing stream
>>     produced on JDK 8 and read with JDK 11. I narrowed the problem
>>     down to
>>     serialization/deserialization of a public EnumSet.class object. There
>>     were several changes made to EnumSet in the Mercurial history of jdk
>>     repo, but I think the following two broke the serialization:
>>
>>     http://hg.openjdk.java.net/jdk/jdk/rev/d0e8542ef650
>>     http://hg.openjdk.java.net/jdk/jdk/rev/a7e13065a7a0
>>
>>     It is interesting to note that before those two changes were made,
>>     there
>>     was a chance to fix the problem reported by newly added serial lint
>>     warnings. Unfortunately they were just silenced:
>>
>>     http://hg.openjdk.java.net/jdk/jdk/rev/501d8479f798
>>
>>     + at SuppressWarnings("serial") // No serialVersionUID due to usage of
>>     +                            // serial proxy pattern
>>
>>     It is true that serialization of instances of Serializable classes is
>>     not broken by changes to them when they implement serial proxy
>>     pattern
>>     (i.e. writeReplace() method) even if they don't itself declare a
>>     private
>>     static final long serialVersionUID field, but this is not true of
>>     Class
>>     objects representing those Serializable classes. It is even more
>>     controversial that serialization of Class objects representing
>>     non-Serializable classes is never broken (which is understandable as
>>     they don't have a habit of declaring serialVersionUID fields).
>>
>>     Both of the above braking changes were made post JDK 8 release, so
>>     deserialization of JDK 8 (and older) streams is affected in all
>>     JDK 9 +
>>     releases or vice versa.
>>
>>     So, what shall be done. I suggest adding serialVersionUID field to
>>     EnumSet vith a value that corresponds to JDK 8 serialization
>>     format and
>>     later backport this change to JDK 11.
>>
>>     What do you think?
>>
>>
>>     Regards, Peter
>>
>>
>>     PS: ImmutableCollections nested classes also implement serial proxy
>>     pattern and don't declare serialVersionUID fields, but they are not
>>     public, so it is less chance that Class objects representing them
>>     could
>>     be used in serial streams, although it is not impossible. For example:
>>
>>     objectOutputStream.writeObject(Set.of().getClass());
>>
>