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

[Mandy Chung]

On 6/18/20 5:43 AM, Aleksey Shipilev wrote:
> On 6/17/20 6:50 PM, Mandy Chung wrote:
>> On 6/17/20 8:11 AM, Aleksey Shipilev wrote:
>>> On 6/15/20 11:58 PM, Mandy Chung wrote:
>>> 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.
> I understand the intent. My point is that intent is mistimed during this time. I understand this
> tripwire needs to be put while Records are still in preview. My point is that it cannot be put in
> before Records serialization story has the preview-proven answer.
>
> The intent also looks rather opaque. The intent is to notify maintainers, fine. I am one of the
> maintainers (although not of serialization library, but still heavy-Unsafe one), so hear me out. I
> came up with the workaround above in about 15 minutes after someone pointed out the exception to me.
> Have it crossed my mind that maybe JDK developers want some help here? No. It looks like another
> impediment that should be solved on the spot. Does that exception communicate any intent at all? The
> message is generic. There is no comment. How would you expect to push maintainers to think along the
> "we should be collaborating to find a proper way to do this", instead of "this is set in stone; let
> me hack this around too"?

Records is in second preview (JEP 384).  As clearly stated in JEP 12 and 
[1],  introducing a feature as a preview feature enables developers to 
provide feedbacks before it becoems permanent in a future Java SE Platform.

We should encourage developers to engage and provide feedbacks and have 
the discussion during preview.

For JOL, have you raised an enhancement request to the serviceability 
team? Inspecting on an object layout would be useful and I suggest you 
file a RFE if you have done so.

[1] 
https://docs.oracle.com/en/java/javase/14/language/preview-language-and-vm-features.html
> This is the actionable bit:
>
> At very least the exception message should say something about the intent. And maybe even the
> comment in Unsafe.java should point to the discussion about this intent and maybe even provide the
> breadcrumbs to follow, e.g. to ObjectInputStream.readRecord().

To hit this exception, the app must be running with --enable-preview.   
There are documentation about preview feature and warning messages 
etc.   To avoid duplicating the info, the best I could do may be to add 
"preview feature" in the exception message?

Chris Hegarty will definitely want to help out w.r.t. the serialization 
frameworks in supporting records.

Mandy