On Thu, 1 Jun 2023 05:51:51 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> It looks like you are right.
>> There is also and assert at line 519:
>> `519 assert(agent->is_loaded(), "invariant");`
>>
>> So, the agent has to be loaded if we got to the line 512.
>> Also, there is a statement at line 507 (before line 512):
>> `507 agent->set_os_lib(library);`
>
>> The use of `!JvmtiAgentList::is_loaded(library)` here is a bit confusing.
>> Isn't the library always already loaded by the time we get here (the assert
>> below seems to imply that)?
>
> JvmtiAgentList::load_agent creates the JvmtiAgent, attempts to load it, and
> adds it to the agent list if is succeeds. The test that you are looking is
> checking the list of already loaded agents. Maybe the function name
> "is_loaded" is the confusion here, maybe a better name is needed.
I see the source of my confusion.
The function `invoke_Agent_OnAttach` is called from the `JvmtiAgent::load`.
But at the time of `invoke_Agent_OnAttach` the agent has not been added to the
list yet.
It is added after the `JvmtiAgent::load` with the function
`JvmtiAgentList::add`:
jint JvmtiAgentList::load_agent(const char* agent_name, const char* absParam,
const char* options, outputStream* st) {
// The abs parameter should be "true" or "false"
const bool is_absolute_path = (absParam != nullptr) && (strcmp(absParam,
"true") == 0);
JvmtiAgent* const agent = new JvmtiAgent(agent_name, options,
is_absolute_path, /* dynamic agent */ true);
if (agent->load(st)) {
add(agent);
. . .
Now I see the code is correct.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13899#discussion_r1212665902
