RFR (XS): 8148044: Remove Enum[0] constants from EnumSet and EnumMap

[Stuart Marks]

On 1/22/16 7:44 AM, Claes Redestad wrote:
>>> It would seem the assessment that serialVersionUID is not needed when
>>> using the serial proxy pattern might be misguided (or is this a bug in
>>> ObjectInputStream)?
>>
>> It is debate-able whether this is a test bug or not. But I think
>> we should not change the serialVersionUID for this class, whether
>> truly Serializable or not. Even though in practice it is not all
>> that useful.
>>
>> Can you please revert the serialization parts of the change, and
>> rerun the test? To see if still passes or fails.
>
> I backed out the serial version UID change and reran the test, same thing.
>
> I dug deeper, as this is a bit vexxing...
>
> While a static field does not in and of itself cause the generated UID to change,
> the private ZERO_LENGTH_ENUM_ARRAY is being accessed from SerializationProxy
> class, thus javac generates a package-private synthetic method (access$000) which
> will be accounted for when generating serialVersionUID. That's unfortunate.
>
> I ran a few experiments to confirm this:
>
> - Adding a package-private static method to EnumSet makes the test fail
> - Adding any static field and the test keep passing
>
> So it seems our options here are limited.

I think this is a bug in the test (BogusEnumSet.java).

EnumSet, a class in the runtime, should never appear in a serialized stream, 
thus it shouldn't have to declare a serialVersionUID in order to remain 
compatible with anything.

The test deliberately attempts to create a serialized stream that should never 
occur in actual use, so the test should be responsible for constructing that 
stream properly. Unfortunately, the test doesn't actually construct that stream; 
instead it's a bunch of hex constants.

It's tedious but not difficult to patch the constants with the modified value. 
I've appended a patch below that does this, along with removing EnumSet's 
serialVersionUID and restoring the @SuppressWarnings.

I can post a webrev if this doesn't come through properly.

s'marks


diff -r c421a3928efc -r 6de800698cf7 
src/java.base/share/classes/java/util/EnumMap.java
--- a/src/java.base/share/classes/java/util/EnumMap.java	Mon Jan 11 10:13:29 
2016 -0800
+++ b/src/java.base/share/classes/java/util/EnumMap.java	Fri Jan 22 10:42:24 
2016 -0800
@@ -25,7 +25,6 @@

  package java.util;

-import java.util.Map.Entry;
  import jdk.internal.misc.SharedSecrets;

  /**
@@ -125,8 +124,6 @@
          return (V)(value == NULL ? null : value);
      }

-    private static final Enum<?>[] ZERO_LENGTH_ENUM_ARRAY = new Enum<?>[0];
-
      /**
       * Creates an empty enum map with the specified key type.
       *
diff -r c421a3928efc -r 6de800698cf7 
src/java.base/share/classes/java/util/EnumSet.java
--- a/src/java.base/share/classes/java/util/EnumSet.java	Mon Jan 11 10:13:29 
2016 -0800
+++ b/src/java.base/share/classes/java/util/EnumSet.java	Fri Jan 22 10:42:24 
2016 -0800
@@ -92,8 +92,6 @@
       */
      final Enum<?>[] universe;

-    private static Enum<?>[] ZERO_LENGTH_ENUM_ARRAY = new Enum<?>[0];
-
      EnumSet(Class<E>elementType, Enum<?>[] universe) {
          this.elementType = elementType;
          this.universe    = universe;
@@ -421,6 +419,9 @@
      private static class SerializationProxy <E extends Enum<E>>
          implements java.io.Serializable
      {
+
+        private static final Enum<?>[] ZERO_LENGTH_ENUM_ARRAY = new Enum<?>[0];
+
          /**
           * The element type of this enum set.
           *
diff -r c421a3928efc -r 6de800698cf7 test/java/util/EnumMap/EnumMapBash.java
--- a/test/java/util/EnumMap/EnumMapBash.java	Mon Jan 11 10:13:29 2016 -0800
+++ b/test/java/util/EnumMap/EnumMapBash.java	Fri Jan 22 10:42:24 2016 -0800
@@ -48,8 +48,6 @@
          bash(Silly500.class);
      }

-    private static Enum[] ZERO_LENGTH_ENUM_ARRAY = new Enum[0];
-
      static <T extends Enum<T>> void bash(Class<T> enumClass) {
          Enum[] universe = enumClass.getEnumConstants();

diff -r c421a3928efc -r 6de800698cf7 test/java/util/EnumSet/BogusEnumSet.java
--- a/test/java/util/EnumSet/BogusEnumSet.java	Mon Jan 11 10:13:29 2016 -0800
+++ b/test/java/util/EnumSet/BogusEnumSet.java	Fri Jan 22 10:42:24 2016 -0800
@@ -32,6 +32,11 @@

  public class BogusEnumSet {
      public static void main(String[] args) throws Throwable {
+        // This test depends on the current serialVersionUID of EnumSet,
+        // which may change if the EnumSet class is modified.
+        // The current value is 4168005130090799668L = 0x39D7BA9531116234L
+        // If the value changes, it will have to be patched into the
+        // serialized byte stream below at the location noted.
          byte[] serializedForm  = {
              (byte)0xac, (byte)0xed, 0x0, 0x5, 0x73, 0x72, 0x0, 0x18,
              0x6a,  0x61,  0x76,  0x61, 0x2e,  0x75,  0x74,  0x69,
@@ -40,9 +45,10 @@
              0x7e, (byte)0xb0, (byte)0xd0, 0x7e, 0x2, 0x0, 0x1, 0x4a, 0x0, 0x8,
              0x65, 0x6c, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x73, 0x78, 0x72, 0x0,
              0x11, 0x6a,  0x61,  0x76,  0x61, 0x2e,  0x75,  0x74,  0x69,
-            0x6c,  0x2e, 0x45, 0x6e, 0x75, 0x6d, 0x53, 0x65, 0x74, 0xe,
-            0x3, 0x21, 0x6a, (byte)0xcd, (byte)0x8c, 0x29, (byte)0xdd, 0x2,
-            0x0, 0x2, 0x4c, 0x0, 0xb, 0x65, 0x6c, 0x65, 0x6d, 0x65, 0x6e, 0x74,
+            0x6c,  0x2e, 0x45, 0x6e, 0x75, 0x6d, 0x53, 0x65, 0x74,
+            // EnumSet's serialVersionUID is the following eight bytes (big-endian)
+            0x39, (byte)0xd7, (byte)0xba, (byte)0x95, 0x31, 0x11, 0x62, 0x34,
+            0x2, 0x0, 0x2, 0x4c, 0x0, 0xb, 0x65, 0x6c, 0x65, 0x6d, 0x65, 0x6e, 
0x74,
              0x54, 0x79, 0x70, 0x65, 0x74, 0x0, 0x11, 0x4c, 0x6a, 0x61, 0x76,
              0x61, 0x2f, 0x6c, 0x61, 0x6e, 0x67, 0x2f, 0x43, 0x6c, 0x61, 0x73,
              0x73, 0x3b, 0x5b, 0x0, 0x8, 0x75, 0x6e, 0x69, 0x76, 0x65, 0x72,