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

[Serguei Spitsyn]

On Wed, 31 May 2023 20:21:15 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 17 additional commits since the last revision:
>> 
>>  - Add impl note to document the XX option
>>  - Cleanup
>>  - Merge
>>  - Allow for warning to be skipped when same agent loaded a second/subsequent time
>>  - Merge
>>  - Tweak javadoc, update test to use more test infra
>>  - Merge
>>  - Merge
>>  - Refresh package description
>>  - Merge
>>  - ... and 7 more: https://git.openjdk.org/jdk/compare/a3c18c96...2d9d5922
>
> src/hotspot/share/prims/jvmtiAgent.cpp line 512:
> 
>> 510: 
>> 511:   // Print warning if EnableDynamicAgentLoading not enabled on the command line
>> 512:   if (!FLAG_IS_CMDLINE(EnableDynamicAgentLoading) && !agent->is_instrument_lib() && !JvmtiAgentList::is_loaded(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)? If so, wouldn't it already be in the list? If it's not in the list yet, perhaps a comment explaining why would be helpful here.

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);`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13899#discussion_r1212584288