changeset 58364:b3e8bdfcb968 records

records: clean up in Serialization code
author chegar
date Mon, 21 Oct 2019 14:41:49 +0100
parents a69ac8d0dc21
children 52a9226db842
files src/java.base/share/classes/java/io/ObjectInputStream.java src/java.base/share/classes/java/io/ObjectStreamClass.java test/jdk/java/io/Serializable/records/WriteReplaceTest.java
diffstat 3 files changed, 48 insertions(+), 36 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/io/ObjectInputStream.java	Sun Oct 20 21:03:40 2019 -0400
+++ b/src/java.base/share/classes/java/io/ObjectInputStream.java	Mon Oct 21 14:41:49 2019 +0100
@@ -26,7 +26,7 @@
 package java.io;
 
 import java.io.ObjectStreamClass.WeakClassKey;
-import java.io.ObjectStreamClass.RecordReflector;
+import java.io.ObjectStreamClass.RecordSupport;
 import java.lang.System.Logger;
 import java.lang.invoke.MethodHandle;
 import java.lang.ref.ReferenceQueue;
@@ -2222,11 +2222,11 @@
 
         FieldValues fieldValues = defaultReadFields(null, desc);
 
-        // lookup the canonical constructor
-        MethodHandle ctrMH = RecordReflector.canonicalCtr(desc.forClass());
+        // retrieve the canonical constructor
+        MethodHandle ctrMH = desc.getRecordConstructor();
 
         // bind the stream field values
-        ctrMH = RecordReflector.bindCtrValues(ctrMH, desc, fieldValues);
+        ctrMH = RecordSupport.bindCtrValues(ctrMH, desc, fieldValues);
 
         try {
             return ctrMH.invoke();
--- a/src/java.base/share/classes/java/io/ObjectStreamClass.java	Sun Oct 20 21:03:40 2019 -0400
+++ b/src/java.base/share/classes/java/io/ObjectStreamClass.java	Mon Oct 21 14:41:49 2019 +0100
@@ -27,7 +27,6 @@
 
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodType;
 import java.lang.ref.Reference;
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.SoftReference;
@@ -192,6 +191,8 @@
 
     /** serialization-appropriate constructor, or null if none */
     private Constructor<?> cons;
+    /** record canonical constructor, or null */
+    private MethodHandle canonicalCtr;
     /** protection domains that need to be checked when calling the constructor */
     private ProtectionDomain[] domains;
 
@@ -520,7 +521,7 @@
                     }
 
                     if (isRecord) {
-                        cons = null;  // ctr will be found later
+                        canonicalCtr = canonicalRecordCtr(cl);
                     } else if (externalizable) {
                         cons = getExternalizableConstructor(cl);
                     } else {
@@ -562,14 +563,16 @@
                 deserializeEx = new ExceptionInfo(name, "no valid constructor");
             }
         }
-        if (!isRecord) {
-        for (int i = 0; i < fields.length; i++) {
-            if (fields[i].getField() == null) {
-                defaultSerializeEx = new ExceptionInfo(
-                    name, "unmatched serializable field(s) declared");
+        if (isRecord && canonicalCtr == null) {
+            new ExceptionInfo(name, "record canonical constructor not found");
+        } else {
+            for (int i = 0; i < fields.length; i++) {
+                if (fields[i].getField() == null) {
+                    defaultSerializeEx = new ExceptionInfo(
+                        name, "unmatched serializable field(s) declared");
+                }
             }
         }
-        }
         initialized = true;
     }
 
@@ -735,6 +738,7 @@
         this.cl = cl;
         if (cl != null) {
             this.isRecord = cl.isRecord();
+            this.canonicalCtr = osc.canonicalCtr;
         }
         this.resolveEx = resolveEx;
         this.superDesc = superDesc;
@@ -1542,6 +1546,33 @@
         return reflFactory.newConstructorForSerialization(cl);
     }
 
+    /** Determines the canonical constructor for the given record class. */
+    @SuppressWarnings("removal")
+    private static MethodHandle canonicalRecordCtr(Class<?> cls) {
+        assert cls.isRecord() : "Expected record, got: " + cls;
+        PrivilegedAction<MethodHandle> pa = () -> {
+            Class<?>[] paramTypes = Arrays.stream(cls.getRecordComponents())
+                                                     .map(RecordComponent::getType)
+                                                     .toArray(Class<?>[]::new);
+            try {
+                Constructor<?> ctr = cls.getConstructor(paramTypes);
+                ctr.setAccessible(true);
+                return MethodHandles.lookup().unreflectConstructor(ctr);
+            } catch (IllegalAccessException | NoSuchMethodException e) {
+                throw new InternalError("should not reach here",  e);
+            }
+        };
+        return AccessController.doPrivileged(pa);
+    }
+
+    /**
+     * Returns the canonical constructor, if the local class equivalent of this
+     * stream class descriptor is a record class, otherwise null.
+     */
+    MethodHandle getRecordConstructor() {
+        return canonicalCtr;
+    }
+
     /**
      * Returns non-static, non-abstract method with given signature provided it
      * is defined by or accessible (via inheritance) by the given class, or
@@ -2468,29 +2499,9 @@
         }
     }
 
-    /** A reflector implementation for record classes. */
+    /** Record specific support for retrieving and binding stream field values. */
     @SuppressWarnings("removal")
-    static final class RecordReflector {
-
-        // TODO: add cache to avoid subsequent reflective calls for the same record class
-
-        /** Returns the canonical constructor for the given record class. */
-        static MethodHandle canonicalCtr(Class<?> cls) {
-            assert cls.isRecord() : "Expected record, got: " + cls;
-            PrivilegedAction<MethodHandle> pa = () -> {
-                Class<?>[] paramTypes = Arrays.stream(cls.getRecordComponents())
-                                               .map(RecordComponent::getType)
-                                               .toArray(Class<?>[]::new);
-                try {
-                    Constructor<?> ctr = cls.getConstructor(paramTypes);
-                    ctr.setAccessible(true);
-                    return MethodHandles.lookup().unreflectConstructor(ctr);
-                } catch (IllegalAccessException | NoSuchMethodException e) {
-                    throw new InternalError("should not reach here",  e);
-                }
-            };
-            return AccessController.doPrivileged(pa);
-        }
+    static final class RecordSupport {
 
         /** Binds the given stream field values to the given method handle. */
         static MethodHandle bindCtrValues(MethodHandle ctrMH,
@@ -2553,7 +2564,8 @@
 
         /**
          * Returns the stream field value for the given name. The default value
-         * for the given type is returned if the field value is absent. */
+         * for the given type is returned if the field value is absent.
+         */
         private static Object streamFieldValue(String pName,
                                                Class<?> pType,
                                                ObjectStreamClass desc,
--- a/test/jdk/java/io/Serializable/records/WriteReplaceTest.java	Sun Oct 20 21:03:40 2019 -0400
+++ b/test/jdk/java/io/Serializable/records/WriteReplaceTest.java	Mon Oct 21 14:41:49 2019 +0100
@@ -24,7 +24,7 @@
 /*
  * @test
  * @summary Basic tests for writeReplace
- * @compile --enable-preview -source 14 StreamRefTest.java
+ * @compile --enable-preview -source 14 WriteReplaceTest.java
  * @run testng/othervm --enable-preview WriteReplaceTest
  * @run testng/othervm/java.security.policy=empty_security.policy --enable-preview WriteReplaceTest
  */