(15) RFR: JDK-8247444: Trust final fields in records

Mandy Chung mandy.chung at oracle.com
Mon Jun 15 21:58:19 UTC 2020


This patch is joint contribution from Christoph Dreis [1] and me.

Webrev: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8247444/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8247517

This proposes to make final fields in records notmodifiable via 
reflection.  Field::set throws IAE if a Field is not modifiable.  
Thecurrent specification specifies the following Fields not modifiable:
- static final fields in any class
- final fields in a hidden class

The spec of Field::set is changed to specify that records are not 
modifiable via reflection.
  Noe that no change in Field::setAccessible(true), i.e. it will succeed 
to allow existing frameworks to have read access to final fields in 
records.  Just no write access.

VarHandle does not support write access if it's a static final field or 
an instance final field.

This patch also proposes `sun.misc.Unsafe::objectFieldOffset` and 
`sun.misc.Unsafe::staticField{Offset/Base}` not to support records.

No change is made in JNI.  JNI could be considered to disallow 
modification of final fields in records and hidden classes (static and 
instance fields).  But JNI has superpower and the current spec already 
allows to modify the value of a final static field even after it's 
constant folded (via JNI Set<type>Field and SetStatic<type>Field), then 
all bets are off.  This should be re-visited when we consider "final is 
truly final" for all classes.

Make final fields in records not modifiable via reflection enables JIT 
optimization as these final fields are effectively truly final.

This change impacts 3rd-party frameworks including 3rd-party 
serialization framework that rely on core reflection `setAccessible` or 
`sun.misc.Unsafe::allocateInstance` and `objectFieldOffset` etc to 
construct records but not using the canonical constructor.
These frameworks would need to be updated to construct records via its 
canonical constructor as done by the Java serialization.

I see this change gives a good opportunity to engage the maintainers of 
the serialization frameworks and work together to support new features 
including records, inline classes and the new serialization mechanism 
and which I think it is worth the investment.

This is a low risk enhancement.  I'd like to request approval for a late 
enhancement in JDK 15.  It extends the pre-existing code path with 
refactoring the hidden classes to prepare for new kinds of classes with 
trusted final fields.  The change is straight-forward.

Can this wait to integrate in JDK 16?

   It's important to get this enhancement in when record is a preview 
feature that we can get feedback and give time to 3rd-party 
serialization frameworks to look into adding the support for records.  
If we delayed this change in 16 and records exit preview, it would be 
bad for frameworks if they verify that they support records in 15 but 
fail in 16.  OTOH the risk of this patch is low.

Performance Impact

I addressed the performance concern I raised earlier myself.  For 
reflection, VM creates the reflective Field objects and fills in 
MemberName when resolving a member.  VM will tag if this 
Field/MemberName is trusted final field.  I think this is a cleaner 
approach rather than in each place to check for final static and final 
fields in hidden or record class to determine if it has write access or 
not.

`sun.misc.Unsafe` does not use Field::isTrustedFinalField because (1) 
Unsafe has been allowing access of static final fields of any classes 
but isTrustedFinalField is not limited to instance fields (2) Unsafe 
disallows access to all fields in a hidden class (not limited to trusted 
final fields).  So it follows the precedence and simply checks if the 
declaring class is a record. `Class::isRecord` calls 
`Class::getSuperclass` to check if it's a subtype of `Record`.  As 
`Class::getSuperclass` is intrinsified, the call on isRecord on a normal 
class is fast. Christoph has contributed the microbenchmarks that 
confirm that no performance regression.

Thanks
Mandy
[1] 
https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-June/040096.html


More information about the amber-dev mailing list