On Wed, 21 Feb 2024 21:13:36 GMT, Alex Menkov <amen...@openjdk.org> wrote:
> VirtualMachine.loadAgentPath/loadAgentLibrary can fail with
> AgentLoadException in 2 cases:
> - attach listener returns error; in the case the exception is thrown from
> HotSpotVirtualMachine.processCompletionStatus (called from
> HotSpotVirtualMachine.execute);
> - attach listener returns success, but reply does not contain Agent_onAttach
> return code ("return code: %d" message).
>
> before jdk21 if attach listener fails to load required library, it returned
> error (case 1)
> from jdk21 attach listener always returns success (case 2)
> Both cases are ok, but for 2nd case HotSpotVirtualMachine.loadAgentLibrary
> read only single line of the response message, so exception message didn't
> contain error details.
>
> The fix updates HotSpotVirtualMachine.loadAgentLibrary to read the whole
> response message.
> Walking through the code, fixed some other minor things in attach listener
> and attach API implementation (commented in the code)
>
> Testing:
> - test/jdk/com/sun/tools;
> - tier1,tier2,tier5-svc
src/hotspot/share/prims/jvmtiAgentList.cpp line 127:
> 125: }
> 126:
> 127: void JvmtiAgentList::add(const char* name, const char* options, bool
> absolute_path) {
`options` parameter should be const
src/hotspot/share/prims/jvmtiAgentList.cpp line 131:
> 129: }
> 130:
> 131: void JvmtiAgentList::add_xrun(const char* name, const char* options,
> bool absolute_path) {
`options` parameter should be const
src/hotspot/share/prims/jvmtiAgentList.cpp line 201:
> 199:
> 200: // Invokes Agent_OnAttach for agents loaded dynamically during runtime.
> 201: jint JvmtiAgentList::load_agent(const char* agent_name, const char*
> absParam,
The method never returns error, dropped value return type
src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java line
402:
> 400: }
> 401:
> 402: // Special-case the "load" command so that the right
> exception is
We had special logic for "load" command in 2 places (here and in the
loadAgentLibrary method)
For simplification moved all logic to loadAgentLibrary()
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17954#discussion_r1498320621
PR Review Comment: https://git.openjdk.org/jdk/pull/17954#discussion_r1498320775
PR Review Comment: https://git.openjdk.org/jdk/pull/17954#discussion_r1498321476
PR Review Comment: https://git.openjdk.org/jdk/pull/17954#discussion_r1498333650
