[11u]: RFR 8227650: EnumSet.class serialization broken in JDK 9+

[Stuart Marks]

On 7/13/19 9:09 AM, Peter Levart wrote:
> This is a review request for JDK-8227368 [1] originally applied to JDK 13, to 
> be backported to JDK 11 update release, tracked by JDK-8227650 [2].
>
> The proposed patch is unchanged at first [3], but since the original patch 
> required CSR [4], it might be necessary to change it a bit for JDK 11 update 
> release. Changes introduced in JDK 9 timeframe broke serialization of 
> EnumSet.class (java.lang.Class<EnumSet>) objects, so this patch is trying to 
> bring back the compatibility across all currently maintained releases (JDK 8, 
> JDK 11 u, JDK 13). There will be no backport to JDK 8 u, since we are 
> restoring the behavior of that release. Nevertheless this is a change to the 
> serialization format and the published specification of it 
> (serialized-form.html), so I'm asking if this is allowed in an update release. 
> If the published specification matters, the patch could be tweaked so that the 
> published serialized-form.html wouldn't change, only the behavior would. But 
> this sounds like cheating. Please advise.
>
> Regards, Peter
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8227368
> [2] https://bugs.openjdk.java.net/browse/JDK-8227650
> [3] 
> http://cr.openjdk.java.net/~plevart/jdk11u-dev/8227650_EnumSet.class_serialization/webrev.01/
> [4] https://bugs.openjdk.java.net/browse/JDK-8227432
>
Hi Peter,

First, thanks for finding and fixing this bug in JDK 13. I think we all learned 
a bunch more than we ever expected to about serialVersionUID!

I've been doing some investigation about how this fix can be applied to the 
backports. A straight backport as you've posted in [3] is indeed a specification 
change. It's not so much a matter of whether it's allowed or disallowed; it's 
indeed possible to make specification changes in update releases. However, doing 
so requires a JCP Maintenance Release (MR) of the corresponding Java SE 
specification JSR.

My understanding is that in the past we (Oracle) have only done MRs when it's 
been absolutely necessary and when there's no alternative. The actual decision 
though is in the hands of the update leads (Andrew Haley for JDK 11, and Rob 
McKenna for JDK 12).

In this message I'll make the case that an MR is not absolutely necessary, and 
I'll outline an alternative.

First, a bit more background.

The essence of the JDK 13 fix is that it adds the line

private static final long serialVersionUID = 1009687484059888093L;

to EnumSet.java. This changes the javadoc output of serialized-form.html. In an 
earlier message [5] I had suggested a "sleazy hack" (or, as Peter put it more 
gently, "cheating"), doing something like the following in order to avoid 
changing serialized-form.html:

private static final long serialVersionUID; static { serialVersionUID = 
1009687484059888093L; }

Indeed, this avoids changing serialized-form.html. However, there's still a 
problem. I ran JCK 11 with this patch applied to my JDK 11 build, and the result 
is that the api/serializabletypes test failed. So, cheating or sleazy hack, this 
approach doesn't really work. Although this technique avoids changing the /text/ 
of the specification output, it really is changing the specification, as far as 
the JCK is concerned.

The good news is that it's possible to change EnumSet's serialVersionUID back to 
what it was in JDK 8, without changing the specification. In effect, I reverted 
the post-JDK 8 changes that had caused changes to the svuid. These were the 
following:

1) A couple fields had been made transient.

I removed the transient modifiers. This has no practical effect on the 
serialized output, since EnumSet uses a serialization proxy. However, it had the 
effect of making those fields documented in serialized-form.html. I was able to 
suppress that by adding a serialPersistentFields field with a zero-length array.

2) The <clinit> method was removed.

The svuid computation includes the presence of a <clinit> (static initializer) 
method on the class. A non-constant expression as the initializer of a static 
field effectively creates a <clinit> even though there's no static block. The 
JDK 8 code initialized one static field with a non-constant expression. A 
post-JDK 8 change removed this, thereby removing <clinit>, thus affecting the 
svuid. The addition of the zero-length array creation for serialPersistentFields 
(above) is sufficient to reintroduce <clinit>.

3) The svuid computation includes synthetic methods, including one named access$000.

The JDK 8 code defined a private field in EnumSet that was referenced from a 
nested class, the serialization proxy. This required the addition of a bridge 
method, in this case named access$000. (This predates nestmates, which removes 
the need for such bridge methods.) A post-JDK 8 change moved this private field 
into the nested class, which removed the need for the bridge method. I was able 
to "revert" the effect of this change by adding a no-op method named access$000. 
Yes, another sleazy hack, my specialty!

**

The webrev for this changeset is [6]. I've adjusted the BogusEnumSet test 
accordingly. I didn't change the EnumSetClassSerialization test from the JDK 13 
patch. This passes the regression tests, the JCK tests, and the 
serialized-form.html is unchanged as far as I can see (except for some 
incidental markup). And of course the svuid matches that of JDK 8 -- which is 
tested by the EnumSetClassSerialization test.

This gives up some of the performance gain introduced by the intervening 
changesets. This probably causes loading of ObjectStreamField.class and 
ObjectStreamField[].class, and it (re-)introduces a static initializer. This is 
unfortunate, but it doesn't seem too terrible to me. (I haven't measured it though.)

What do you think?

s'marks


[5] http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-June/061114.html

[6] http://cr.openjdk.java.net/~smarks/reviews/8227368.jdk11u/webrev.0/