Hi Stuart,
I managed to find some time to tackle this bug.
What I found from inspecting the JDK code and experimenting is that it
doesn't matter how the private static final long serialVersionUID field
is initialized. Whether it is a compile-time constant or its value is
computed in <clinit>, the serialization/deserialization infrastructure
acts the same. It is using reflection to obtain the field's value and so
it can't distinguish between compile-time constant and computed value.
So ypur plan to eliminate serialVersionUID value from the stream of
"patched" EnumSet.class object falls apart.
I filed a bug [1] and prepared a patch [2] which just adds
serialVersionUID to EnumSet and initializes it to the same value that is
automatically computed in JDK 8 and before.
I had to revert the existing BogusEnumSet test to basically a version
that was actual at the time of JDK 8 release, undoing the edits of
byte[] value representing serialized stream that have been performed by
the same patches that broke the serialization of EnumSet.class objects.
I also added a test that verifies the de-serialization of a stream
containing EnumSet.class object, produced on JDK 8.
Although the patched EnumSet now contains explicit serialVersionUID,
serializing EnumSet.class with patched JDK 14 does not procude the same
stream, because the stream also contains descriptors of EnumSet fields
which changed from version in JDK 8. When deserializing EnumSet.class
object, only serialVersionUID from stream is checked against local
version though, so this doesn't matter.
Considering that this patch breaks serialization between already
released 9,10,11,12 versions of JDK and yet to be released versions that
would contain the patch, I think the patch should be backported to:
- 13
- 11u (11.0.4?)
(and possibly to other 9+ update versions yet to be released).
This would provide consistent serialization among latest releases of
actual versions including JDK 8 and before.
Do you think it is necessary to provide a fall-back system property or
just advise people to upgrade to latest update release in case they
experience trouble? Upgrading to latest update releases is pretty safe
these days and should not be a problem for anyone.
Regards,
Peter
[1] https://bugs.openjdk.java.net/browse/JDK-8227368
[2]
http://cr.openjdk.java.net/~plevart/jdk-dev/8227368_EnumSet.class_serialization/
On 6/29/19 2:00 AM, Stuart Marks wrote:
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
*/
-@SuppressWarnings("serial") // No serialVersionUID due to usage of
- // serial proxy pattern
+@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.lev...@gmail.com <mailto:peter.lev...@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
+@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());
