RFR: JDK-8325530: Vague error message when com.sun.tools.attach.VirtualMachine fails to load agent library

[Alex Menkov]

On Wed, 21 Feb 2024 21:13:36 GMT, Alex Menkov <amenkov at 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