JDK 14 RFR of JDK-8231202: Suppress warnings on non-serializable non-transient instance fields in serializable classes

Roger Riggs Roger.Riggs at oracle.com
Thu Sep 19 19:13:30 UTC 2019


Hi Joe,

The addition of @SuppressWarnings(serial) hides a number of instances of
poor choices of types.  Before they dissappear behind the suppress warnings,
the fix should be to correct the types used.

For example, in the serialization proxies for java.time, the Ser class 
carelessly
has a field of type Object when it could/should be using Serializable.
The types being serialized and deserialized are known to be Serializable.
See the attach patches for details and a suggested fix.

Thanks, Roger

(p.s. the patch is attached twice, once as .patch and the other .txt.
I'd like to see if they both get through the mail).

On 9/18/19 5:38 PM, Joe Darcy wrote:
> Hello,
>
> As background, I'm working on a number of serialization-related 
> compile-time checks with the goal of enabling stricter javac lint 
> checking in the JDK build (and elsewhere).
>
> One check is tracked by
>
>     JDK-8160675: Issue lint warning for non-serializable non-transient 
> instance fields in serializable type
>
> As summarized in the bug description, it may be concerning if a 
> serializable class has non-transient instance fields whose types are 
> not Serializable. This can cause a serialization failure at runtime. 
> (Classes using the serialPersistentFields mechanism are excluded from 
> this check.)
>
> A common example is an exception type -- all Throwable's are 
> Serializable -- which has a non-serializable field. If the fields 
> cannot be marked as transient, one approach to handle this robustly is 
> to have a writeObject method which null outs the field in question 
> when serializing and make the other methods in the exception 
> null-tolerant.
>
> In other cases, the object pointed to by the non-serializable field 
> are conditionally serializable at runtime. This is the case for many 
> collection types. For example, a class may have a field of type 
> List<Foo> with the field set to an ArrayList<Foo> at runtime. While 
> the List interface does not extent Serializable, the ArrayList class 
> does implement Serializable and the class would serialize fine in 
> practice, assuming the Foo's were serialazable.
>
> As a precursor to the adding a compile-time check to the build, please 
> review adding @SuppressWarnings("serial") to document the 
> non-serializable fields in the core libraries:
>
>     JDK-8231202: Suppress warnings on non-serializable non-transient 
> instance fields in serializable classes
>     http://cr.openjdk.java.net/~darcy/8231202.0/
>
> Bugs for similar changes to client libs and security libs will be 
> filed and reviewed separately.
>
> A more complete fix would add readObject/writeObject null handling to 
> AnnotationTypeMismatchExceptionProxy, but since this hasn't seemed to 
> be an issue since the type was introduced back in JDK 5.0, I just 
> added the annotation, as done elsewhere.
>
> Thanks,
>
> -Joe
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: Ser-serializable.patch
Type: text/x-patch
Size: 7973 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20190919/fc9c7ca7/Ser-serializable-0001.patch>
-------------- next part --------------
# HG changeset patch
# Parent  aad3466779bbcc2778d039ef7aee01ac75228b86

diff -r aad3466779bb -r f584c4228c70 src/java.base/share/classes/java/lang/invoke/SerializedLambda.java
--- a/src/java.base/share/classes/java/lang/invoke/SerializedLambda.java	Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/lang/invoke/SerializedLambda.java	Thu Sep 19 15:04:48 2019 -0400
@@ -77,7 +77,7 @@ public final class SerializedLambda impl
     private final int implMethodKind;
     private final String instantiatedMethodType;
     @SuppressWarnings("serial") // Not statically typed as Serializable
-    private final Object[] capturedArgs;
+    private final Serializable[] capturedArgs;
 
     /**
      * Create a {@code SerializedLambda} from the low-level information present
@@ -114,7 +114,7 @@ public final class SerializedLambda impl
                             String implMethodName,
                             String implMethodSignature,
                             String instantiatedMethodType,
-                            Object[] capturedArgs) {
+                            Serializable[] capturedArgs) {
         this.capturingClass = capturingClass;
         this.functionalInterfaceClass = functionalInterfaceClass;
         this.functionalInterfaceMethodName = functionalInterfaceMethodName;
diff -r aad3466779bb -r f584c4228c70 src/java.base/share/classes/java/time/Ser.java
--- a/src/java.base/share/classes/java/time/Ser.java	Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/Ser.java	Thu Sep 19 15:04:48 2019 -0400
@@ -61,6 +61,7 @@ import java.io.IOException;
 import java.io.InvalidClassException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
+import java.io.Serializable;
 import java.io.StreamCorruptedException;
 
 /**
@@ -112,8 +113,7 @@ final class Ser implements Externalizabl
     /** The type being serialized. */
     private byte type;
     /** The object being serialized. */
-    @SuppressWarnings("serial") // Not statically typed as Serializable
-    private Object object;
+    private Serializable object;
 
     /**
      * Constructor for deserialization.
@@ -127,7 +127,7 @@ final class Ser implements Externalizabl
      * @param type  the type
      * @param object  the object
      */
-    Ser(byte type, Object object) {
+    Ser(byte type, Serializable object) {
         this.type = type;
         this.object = object;
     }
@@ -248,12 +248,12 @@ final class Ser implements Externalizabl
         object = readInternal(type, in);
     }
 
-    static Object read(ObjectInput in) throws IOException, ClassNotFoundException {
+    static Serializable read(ObjectInput in) throws IOException, ClassNotFoundException {
         byte type = in.readByte();
         return readInternal(type, in);
     }
 
-    private static Object readInternal(byte type, ObjectInput in) throws IOException, ClassNotFoundException {
+    private static Serializable readInternal(byte type, ObjectInput in) throws IOException, ClassNotFoundException {
         switch (type) {
             case DURATION_TYPE: return Duration.readExternal(in);
             case INSTANT_TYPE: return Instant.readExternal(in);
diff -r aad3466779bb -r f584c4228c70 src/java.base/share/classes/java/time/chrono/AbstractChronology.java
--- a/src/java.base/share/classes/java/time/chrono/AbstractChronology.java	Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/chrono/AbstractChronology.java	Thu Sep 19 15:04:48 2019 -0400
@@ -731,7 +731,7 @@ public abstract class AbstractChronology
      */
     @java.io.Serial
     Object writeReplace() {
-        return new Ser(Ser.CHRONO_TYPE, this);
+        return new Ser(Ser.CHRONO_TYPE, (Serializable)this);
     }
 
     /**
diff -r aad3466779bb -r f584c4228c70 src/java.base/share/classes/java/time/chrono/Ser.java
--- a/src/java.base/share/classes/java/time/chrono/Ser.java	Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/chrono/Ser.java	Thu Sep 19 15:04:48 2019 -0400
@@ -61,6 +61,7 @@ import java.io.IOException;
 import java.io.InvalidClassException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
+import java.io.Serializable;
 import java.io.StreamCorruptedException;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
@@ -110,8 +111,7 @@ final class Ser implements Externalizabl
     /** The type being serialized. */
     private byte type;
     /** The object being serialized. */
-    @SuppressWarnings("serial") // Not statically typed as Serializable
-    private Object object;
+    private Serializable object;
 
     /**
      * Constructor for deserialization.
@@ -125,7 +125,7 @@ final class Ser implements Externalizabl
      * @param type  the type
      * @param object  the object
      */
-    Ser(byte type, Object object) {
+    Ser(byte type, Serializable object) {
         this.type = type;
         this.object = object;
     }
@@ -231,11 +231,12 @@ final class Ser implements Externalizabl
         return readInternal(type, in);
     }
 
-    private static Object readInternal(byte type, ObjectInput in) throws IOException, ClassNotFoundException {
+    private static Serializable readInternal(byte type, ObjectInput in) throws IOException,
+            ClassNotFoundException {
         switch (type) {
-            case CHRONO_TYPE: return AbstractChronology.readExternal(in);
-            case CHRONO_LOCAL_DATE_TIME_TYPE: return ChronoLocalDateTimeImpl.readExternal(in);
-            case CHRONO_ZONE_DATE_TIME_TYPE: return ChronoZonedDateTimeImpl.readExternal(in);
+            case CHRONO_TYPE: return (Serializable)AbstractChronology.readExternal(in);
+            case CHRONO_LOCAL_DATE_TIME_TYPE: return (Serializable)ChronoLocalDateTimeImpl.readExternal(in);
+            case CHRONO_ZONE_DATE_TIME_TYPE: return (Serializable)ChronoZonedDateTimeImpl.readExternal(in);
             case JAPANESE_DATE_TYPE:  return JapaneseDate.readExternal(in);
             case JAPANESE_ERA_TYPE: return JapaneseEra.readExternal(in);
             case HIJRAH_DATE_TYPE: return HijrahDate.readExternal(in);
diff -r aad3466779bb -r f584c4228c70 src/java.base/share/classes/java/time/zone/Ser.java
--- a/src/java.base/share/classes/java/time/zone/Ser.java	Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/zone/Ser.java	Thu Sep 19 15:04:48 2019 -0400
@@ -68,6 +68,7 @@ import java.io.IOException;
 import java.io.InvalidClassException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
+import java.io.Serializable;
 import java.io.StreamCorruptedException;
 import java.time.ZoneOffset;
 
@@ -97,8 +98,7 @@ final class Ser implements Externalizabl
     /** The type being serialized. */
     private byte type;
     /** The object being serialized. */
-    @SuppressWarnings("serial") // Not statically typed as Serializable
-    private Object object;
+    private Serializable object;
 
     /**
      * Constructor for deserialization.
@@ -112,7 +112,7 @@ final class Ser implements Externalizabl
      * @param type  the type
      * @param object  the object
      */
-    Ser(byte type, Object object) {
+    Ser(byte type, Serializable object) {
         this.type = type;
         this.object = object;
     }
@@ -184,12 +184,13 @@ final class Ser implements Externalizabl
         object = readInternal(type, in);
     }
 
-    static Object read(DataInput in) throws IOException, ClassNotFoundException {
+    static Serializable read(DataInput in) throws IOException, ClassNotFoundException {
         byte type = in.readByte();
         return readInternal(type, in);
     }
 
-    private static Object readInternal(byte type, DataInput in) throws IOException, ClassNotFoundException {
+    private static Serializable readInternal(byte type, DataInput in) throws IOException,
+            ClassNotFoundException {
         switch (type) {
             case ZRULES:
                 return ZoneRules.readExternal(in);


More information about the core-libs-dev mailing list