RFR: 8275338: Add JFR events for notable serialization situations [v10]
    Roger Riggs 
    rriggs at openjdk.org
       
    Thu Dec 21 21:49:55 UTC 2023
    
    
  
On Thu, 21 Dec 2023 09:53:10 GMT, Raffaello Giulietti <rgiulietti at openjdk.org> wrote:
>> Adds serialization misdeclaration events to JFR.
>
> Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Corrected @Label of event and of field.
src/java.base/share/classes/java/io/ObjectStreamClass.java line 466:
> 464: 
> 465:         if (SerializationMisdeclarationEvent.enabled() && serializable) {
> 466:             new SerializationMisdeclarationChecker(cl).checkMisdeclarations();
Is there any benefit to avoiding the allocation and passing the class through the methods as an extra arg?
src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java line 149:
> 147:         if (!(spf instanceof ObjectStreamField[])) {
> 148:             commitEvent(SERIAL_PERSISTENT_FIELDS_NAME +
> 149:                     " must be an instance of ObjectStreamField[] to be effective");
Hmmm...  
The terminology "to be effective" seems a bit indirect and may lead to some head scratching.
In most cases it means it is ignored.
I might suggest to just state the condition that is required and drop the extra phrase.
For example,  "xxx should be an instance of ObjectStreamField[]".
It would result in shorter messages and if further explanation of the message is needed it can more completely elaborate on the impact of the incorrect declaration.
src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java line 165:
> 163:             commitEvent("method " + m + " on an enum class is not effective");
> 164:         } else if (cl.isRecord()) {
> 165:             commitEvent("method " + m + " on an record class is not effective");
"an record" -> "a record"
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1434524764
PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1434512690
PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1434513714
    
    
More information about the security-dev
mailing list