On Mon, 12 May 2025 14:46:44 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:

>> https://openjdk.org/jeps/483 mentions:
>> 
>>> To enjoy the benefits of the AOT cache generated during a training run, the 
>>> training run and all subsequent runs must be essentially similar. [...] All 
>>> runs must not use JVMTI agents that can arbitrarily rewrite classfiles  
>>> using ClassFileLoadHook.
>> 
>> However, when *any* java agent is specified in the training run, the JVM 
>> fails at start-up. E.g.,
>> 
>> 
>> $ java -XX:AOTMode=record -javaagent:agent.jar -cp app.jar App
>> Error occurred during CDS dumping
>> Must enable AllowArchivingWithJavaAgent in order to run Java agent during 
>> CDS dumping
>> 
>> 
>> With the AOT cache, the main concern for JVMTI agents is that they can 
>> modify the contents of loaded Java classes. If we store such modified 
>> classes into the AOT cache, their contents will no longer match the original 
>> class files (from application JAR files, etc). As a result, when using the 
>> AOT cache in production runs, the application may have unexpected behavior.
>> 
>> With this PR, we allow JVMTI agents in the AOT workflow. To address the 
>> above concern, we ensure that JVMTI agents cannot affect the contents of AOT 
>> cache:
>> 
>> -  In training runs (`java -XX:AOTMode=record`), JVMTI agents are allowed, 
>> but the AOT configuration file should filter out classes that are 
>> transformed by the agents. This can be checking 
>> `InstanceKlass::has_been_redefined()` and 
>> `ClassFileStream::from_class_file_load_hook()`.
>> 
>> - In the assembly phase (`java -XX:AOTMode=record`), agents can be specified 
>> in the command-line. However, since the assembly phase doesn't execute any 
>> application logic, we will also not load any of the specified agents. 
>> Therefore, the agents cannot affect the contents of the AOT cache created in 
>> the assembly phase.
>
> src/hotspot/share/prims/jvmtiAgent.cpp line 588:
> 
>> 586:     // Agents are allowed with -XX:AOTMode=record and 
>> -XX:AOTMode=on/auto.
>> 587:     // Agents are completely disabled when -XX:AOTMode=create
>> 588:     assert(!CDSConfig::is_dumping_final_static_archive(), "agents 
>> should have been disabled with -XX:AOTMode=create");
> 
> Is there a point in asserting this? I am thinking users would pass the same 
> command line to `-XX:AOTMode=create` without thinking twice. And since we are 
> doing the right thing, ignoring the agents, it seems we "only" need to print 
> the user-visible warning. Maybe not even that, and just return?

If someone removes the call to `JvmtiAgentList::disable_agent_list()`, we will 
come to here and exit the JVM. This assert is to guard against that.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25170#discussion_r2085127224

Reply via email to