RFR: 8275338: Add JFR events for notable serialization situations [v12]

[Raffaello Giulietti]

On Mon, 8 Jan 2024 18:38:52 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 15 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8275338
>>  - Simplified event messages.
>>    Remove ckecker allocation.
>>  - Corrected @Label of event and of field.
>>  - Removed @module from test.
>>  - Merge branch 'master' into 8275338
>>  - Renamed an event field.
>>  - Minor changes.
>>  - Removed event kind.
>>    serialVersionUID must have type long.
>>    Test now base on keyword search in event message.
>>    Commented test classes about misdeclarations.
>>  - Changes according to reviewer's comments.
>>  - Better name for a label, corrected name of removed field.
>>  - ... and 5 more: https://git.openjdk.org/jdk/compare/09cef802...9ca1f36d
>
> src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java line 70:
> 
>> 68:             privilegedCheckAccessibleMethod(cl, WRITE_REPLACE_NAME,
>> 69:                     WRITE_REPLACE_PARAM_TYPES, Object.class);
>> 70:             privilegedCheckAccessibleMethod(cl, READ_RESOLVE_NAME,
> 
> Thinking ahead to when the security manager is removed, can the code that needs private access to field values (SUID) be more narrowly accessed? Perhaps switch to using a VarHandle and MethodHandles.Lookup. This may be a longer term issue and beyond the scope of this PR.
> 
> In the naming of the `PrivilegedXXX` methods, I would drop the "privileged" part of the name.
> The methods do not change the privilege level and operate correctly if when the caller has the privileges needed. And all of these methods are read-only so there is no/little risk in their implementations and avoid refitting the terminology later.

They are called `privilegedXXX` because they _are_ (already) privileged, not because they change the privileges.

But yes, in view of removing the security manager, the implementation could be more "modern". Will have a look.

> test/jdk/jdk/jfr/event/io/TestSerializationMisdeclarationEvent.java line 33:
> 
>> 31: import org.junit.jupiter.params.provider.MethodSource;
>> 32: 
>> 33: import java.io.*;
> 
> Explicit imports are preferred.

Oops, this is the (overridable) IDE default choice

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1445928358
PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1445930028