xuyangzhong opened a new pull request, #24890:
URL: https://github.com/apache/flink/pull/24890

   ## What is the purpose of the change
   
   Currently, we depend on asm to extract method parameter names. We custom a 
MethodVisitor to visit local variable tables in  file `.class`, and then cut 
the first `N` local variable names as the method parameter names. 
   
   However, if there are multi blocks about one local variable, the first `N` 
local variable names could be same, and then wrongly be extracted, and crashed 
when validating the conflict of them(the new logic added in FLINK-1.19).
   
   This pr will use the slot index in `.class` file to extract the method 
parameter names, because method parameter names are always at the head in the 
'slot index' 
list(https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-3.html)
   
   Take the test function 
`ExtractionUtilsTest#MultiLocalVariableBlocksWithoutInitializationClass` as an 
example:
   
   - Before fix:
   1. extract parameter names from asm: 
   [localVariable, localVariable, localVariable, this, generic, genericFuture, 
listOfGenericFuture, array, localVariable]
   2. get the first 4 after offset 1(expected `this` in index 0)
   [localVariable, localVariable, this, generic]
   
   This is because in local variable table, there are multi `localVariable` 
with different lifecycle in different blocks.
   
   - After fix:
   1. extract parameter names from asm: 
   [this, generic, genericFuture, listOfGenericFuture, array, localVariable]
   2. get the first 4 after offset 1(expected `this` in index 0)
   [generic, genericFuture, listOfGenericFuture, array]
   
   That's what we expected.
   
   ## Brief change log
   
   *(for example:)*
     - *Fix the logic in ExtractionUtils*
     - *Add tests to verify this fix*
   
   
   ## Verifying this change
   
   Some tests are added to verity it.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API, i.e., is any changed class annotated with 
`@Public(Evolving)`: no
     - The serializers: no
     - The runtime per-record code paths (performance sensitive): no
     - Anything that affects deployment or recovery: JobManager (and its 
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
     - The S3 file system connector: no
   
   ## Documentation
   
     - Does this pull request introduce a new feature? no
     - If yes, how is the feature documented? 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to