RFR: 8333796: Add missing serialization functionality to sun.reflect.ReflectionFactory

[David M. Lloyd]

On Thu, 27 Jun 2024 08:08:13 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Issue [JDK-8164908](https://bugs.openjdk.org/browse/JDK-8164908) added support for functionality required to continue to support IIOP and custom serializers in light of additional module-based restrictions on reflection. It was expected that these libraries would use `sun.misc.Unsafe` in order to access fields of serializable classes. However, with JEP 471, the methods necessary to do this are being removed.
>> 
>> To allow these libraries to continue to function, it is proposed to add two methods to `sun.reflect.ReflectionFactory` which will allow serialization libraries to acquire a method handle to generated `readObject`/`writeObject` methods which set or get the fields of the serializable class using the serialization `GetField`/`PutField` mechanism. These generated methods should be used by serialization libraries to serialize and deserialize classes which do not have a `readObject`/`writeObject` method or which use `ObjectInputStream.defaultReadObject`/`ObjectOutputStream.defaultWriteObject` to supplement default serialization.
>> 
>> It is also proposed to add methods which allow for the reading of serialization-specific private static final fields from classes which have them.
>> 
>> With the addition of these methods, serialization libraries no longer need to rely on `Unsafe` for serialization/deserialization activities.
>> cc: @AlanBateman
>
> src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java line 448:
> 
>> 446:      * @since 24
>> 447:      */
>> 448:     public static MethodHandle fieldSetterForSerialization(MethodHandles.Lookup lookup, String name, Class<MethodHandle> type, Class<?> cl) {
> 
> Is this left over from an earlier prototype? There shouldn't be any changes to the standard APIs in this proposal.

I'll look into some alternatives.

> src/java.base/share/classes/java/lang/invoke/SerializationBytecodeGenerator.java line 43:
> 
>> 41:  * The private serialization bytecode generator used by {@code sun.misc.ReflectionFactory}.
>> 42:  */
>> 43: final class SerializationBytecodeGenerator {
> 
> I assume the starting point for the prototype should be a supporting class in jdk.internal.reflect rather than here.

Indeed, if you look at the previous revision, that's how I implemented it. However, the generator currently needs access to `Lookup.IMPL_LOOKUP` in order to generate a nestmate class and accessors for private members on the class. If you look at the previous commit ([right here, for example](https://github.com/openjdk/jdk/compare/430b5d3b618154a66f03acf4d7a095c046a89687%5E..4e68264689bbbf8e5cf2876ac8dc80e4e7f44f6c#diff-c11ce0e1cedb25e3733a80cd7cb308506d63805ada55b4265252f653df123d88R584-R597)), you can see that I _was_ using reflection to grab this field, but that seems less than ideal (I couldn't find anywhere else in the JDK which does this).

My next thought was to expose that lookup via `JavaLangInvokeAccess`, however looking at the history of this interface gave me an impression that this is explicitly undesirable. So my conclusion was that the safest option is to push the generator to the "other side" of the barrier and access the generators themselves via `JavaLangInvokeAccess`, along the lines of what `MethodHandles.Lookup.serializableConstructor` is doing (though in that case there is no code being generated).

Another idea I had this morning was that perhaps I could avoid needing the private lookup at all, by using reflection with `setAccessible(true)` for every serializable field, and unreflecting the result to get getter/setter handles, which should bypass access checks. I'll give that a shot and see if it works, because in that case we don't need any JLI privileged stuff at all. It's a little more clunky than using getfield/putfield but maybe the benefits outweigh the difficulties.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19702#discussion_r1657096322
PR Review Comment: https://git.openjdk.org/jdk/pull/19702#discussion_r1658720910