EnumSet.class serialization broken - twice
Stuart Marks
stuart.marks at oracle.com
Sat Jun 29 00:00:13 UTC 2019
Daniel Fuchs pointed me to a weird thing they had to do with the
serialVersionUID in one of the JMX classes:
https://hg.openjdk.java.net/jdk/jdk/file/c59f36ed7b52/src/java.management/share/classes/javax/management/MBeanAttributeInfo.java#l46
Essentially the svuid for this class initialized in a static block, and its
value is conditional based on the value of some system property. I don't think
using a property is necessary for the EnumSet case. However, it does point out
something interesting, which is that if the svuid is not initialized with a
compile-time constant, and instead via a static block, the value doesn't appear
in serialized-form.html.
Thus, we can backport a change to JDK 11 that changes EnumSet's svuid, without
changing the Java SE 11 specification! (This is analogous to changing a method's
implementation to behave the way we want to, without changing the method's
specification to specify that it behaves that way. On the other hand, this is a
really sleazy hack.)
Here's an outline of what we can do:
1) Add EnumSet.serialVersionUID to JDK 13 in the usual way, specifying a
constant that's the same as the JDK 8 value. We are after RDP1 but I think this
change is well-justified. This change should automatically be propagated to JDK 14.
2) "Backport" the fix to JDK 11, but assign the value in a static initializer
block instead of as a constant in a field initializer. Something like this:
diff -r 27d4b8acbe07 src/java.base/share/classes/java/util/EnumSet.java
--- a/src/java.base/share/classes/java/util/EnumSet.java Thu Jun 13 17:46:57
2019 -0700
+++ b/src/java.base/share/classes/java/util/EnumSet.java Fri Jun 28 16:42:03
2019 -0700
@@ -76,11 +76,18 @@
* @since 1.5
* @see EnumMap
*/
- at SuppressWarnings("serial") // No serialVersionUID due to usage of
- // serial proxy pattern
+ at SuppressWarnings("serial") // serialVersionUID is not a compile-time constant
public abstract class EnumSet<E extends Enum<E>> extends AbstractSet<E>
implements Cloneable, java.io.Serializable
{
+ // Initialize the serialVersionUID to be compatible with JDK 8.
+ // Do this from a static block to avoid having the value appear
+ // in serialized-form.html. See JDK-xxxxxxx.
+ private static final long serialVersionUID;
+ static {
+ serialVersionUID = 1009687484059888093L;
+ }
+
/**
* The class of all the elements of this set.
*/
3) Backport the fix to JDK 12. I'm not sure this is absolutely necessary, since
JDK 12 likely has a short lifetime, but if we're doing 11 and 13 it makes sense
to do 12 as well (though mainly for completeness).
4) It's unclear whether a similar patch as above needs to be added to JDK 8.
Since it already has the right svuid, we could get away without doing anything.
However, with backports continuing in the 8u release family, it might be prudent
to apply this patch in order to prevent future backports to 8u from
inadvertently changing the svuid -- which as you point out did happen in 9 and 10.
5) I don't think we need to patch JDK 1.6, 7, 9, or 10 but the maintainers of
those releases can certainly decide to do so.
=====
In any case, I think doing the above will result in consistent
EnumSet.serialVersionUID values for JDK 8 LTS, JDK 11 LTS, and current JDK
releases, without having to worry about spec changes for any of the past releases.
Let me know how you'd like to proceed. I'm happy to help out with reviewing,
filing bugs, CSRs, etc.
s'marks
On 6/27/19 2:57 PM, Stuart Marks wrote:
> 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());
>>>
>>
More information about the core-libs-dev
mailing list