RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v7]

[Serguei Spitsyn]

On Thu, 1 Jun 2023 05:51:51 GMT, Alan Bateman <alanb at 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