On Tue, 18 Mar 2025 16:54:51 GMT, Jiangli Zhou <jian...@openjdk.org> wrote:

>> Please review this fix that avoids `JvmtiAgent::convert_xrun_agent` from 
>> prematurely exiting VM if `lookup_On_Load_entry_point` cannot load the agent 
>> using `JVM_OnLoad` symbol. Thanks
>> 
>> `lookup_On_Load_entry_point` first tries to load the builtin agent from the 
>> executable by checking the requested symbol (`JVM_OnLoad`). If no builtin 
>> agent is found, it then tries to load the agent shared library (e.g. 
>> `libjdwp.so`) by calling `load_library`. The issue is that `load_library` is 
>> called with `vm_exit_on_error` set to `true`, which causes the VM to exit 
>> immediately if the agent shared library is not loaded. Therefore, 
>> `JvmtiAgent::convert_xrun_agent` has no chance to try loading the agent 
>> using `Agent_OnLoad` symbol 
>> (https://github.com/openjdk/jdk/blob/19154f7af34bf6f13d61d7a9f05d6277964845d8/src/hotspot/share/prims/jvmtiAgent.cpp#L352).
>>  This is a hidden issue on regular JDK, since the `load_library` can 
>> successfully find the agent shared library when 
>> `JvmtiAgent::convert_xrun_agent` first tries to load the agent using 
>> `JVM_OnLoad` symbol. The issue is noticed on static JDK as there is no 
>> `libjdwp.so` in static JDK. It can be reproduced with jtreg 
>> `runtime/6294277/So
 urceDebugExtension.java` test.
>> 
>> As part of the fix, I cleaned up following in `invoke_JVM_OnLoad` and 
>> `invoke_Agent_OnLoad`. If there's an error, the VM should already have 
>> exited during `lookup_<JVM|Agent>_OnLoad_entry_point` in those cases.
>> 
>> 
>>   if (on_load_entry == nullptr) {
>>     vm_exit_during_initialization("Could not find ... function in -Xrun 
>> library", agent->name());
>>   }
>
> Jiangli Zhou has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Apply @dholmes-ora's edit suggestion.
>    
>    Co-authored-by: David Holmes 
> <62092539+dholmes-...@users.noreply.github.com>
>  - Apply @dholmes-ora's suggestion to use single line.
>    
>    Co-authored-by: David Holmes 
> <62092539+dholmes-...@users.noreply.github.com>

I had a look yesterday and it looked good, and your latest changes look good 
also.

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

Marked as reviewed by cjplummer (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24086#pullrequestreview-2695605225

Reply via email to