On 6/17/20 8:11 AM, Aleksey Shipilev wrote:
On 6/15/20 11:58 PM, Mandy Chung wrote:
This patch also proposes `sun.misc.Unsafe::objectFieldOffset` and
`sun.misc.Unsafe::staticField{Offset/Base}` not to support records.
Note this would break otherwise innocuous introspection for records, for
example dumping the VM
layout with JOL:
http://openjdk.java.net/projects/code-tools/jol/
To me, JOL needs a supported API in the serviceability area to
introspect a field layout. JOL will not work for inline types without
adding new APIs.
sun.misc.Unsafe is an unsupported API. My proposal for sun.misc.Unsafe
(in fact jdk.unsupported module) is to keep the support for legacy use
but not necessarily for new features.
Amusingly, with the rest of Unsafe available the barn is still full open.
Here's the sketch for the
workaround: get the field list with getDeclaredFields, optionally guess the VM
layout (the rules are
not that hard, and JOL does it routinely), instantiate a Record with whatever
arguments, poke the
test patterns with Unsafe.put*, read them back from record components to
verify. It would get me the
same objectFieldOffset in a round-about way.
I'm aware of these workarounds in the wild. As sun.misc.Unsafe is JDK
unsupported API, this patch does not attempt to implement a complete
solution but adds an obvious notification informing the frameworks that
`sun.misc.Unsafe::objectFieldOffset` and
`sun.misc.Unsafe::staticField{Offset/Base}` not to support records.
Unsafe is unsafe! Use it at your own risk.
And speaking from JOL maintainer standpoint, that one would be very tempting to
do, because it would
not depend on whatever protection shenanigans a particular JDK release tries to
enforce.
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.
Are we absolutely sure that what ObjectInputStream.readRecord() does fits the
3rd party
serialization libraries? Does it work for them performance-wise?
There should be performance improvement opportunity (see Peter Levart's
good work - JDK-8247532 [1])
(Are we even sure about that for JDK itself?)
Java serialization support for records use its canonical constructor.
(I'm not sure what you tried to point out by this).
Because if it is not, 3rd party lib maintainers would proceed to hacking in
the
"objectFieldOffset workaround" and we would get the cobra-effect-like
strengthening of dependency on
Unsafe quirks.
This is exactly why we request this for 15 so that 3rd-party lib
maintainers can prototype and send feedback. We love to understand any
stumbling block and work together to resolve any issue before records
exit preview.
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.
It is fun to consider JNI to be more powerful than Unsafe. This seems
backwards. The intent to break
Unsafe users might be defensible, but this power oddity is still quite odd.
I think you misread the message. There is no claim whether JNI or
Unsafe is more powerful.
JNI is a supported API. The above explains why I propose no spec
change to make in JNI. OTOH jdk.unsupported is unsupported but has more
love by frameworks to avoid writing in native. Adding a check in some
sun.misc.Unsafe APIs (even it can be hacked to workaround it) is a clear
notification what it does not intend to support.
Hope this helps.
Mandy
[1]
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-June/067223.html
