OpenJDK / amber / amber
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 */