RFR: 8277444: Race condition on Instrumentation.retransformClasses() and class linking

[Evgeny Astigeevich]

On Fri, 22 Aug 2025 06:42:48 GMT, David Holmes <dholmes at openjdk.org> wrote:

>>> @eastig I am not sure about this one. Can you clarify please how you can be transforming a class that has not yet been linked? If this is possible then it seems to me we are missing a call to ensure linkage.
>> 
>> Hi @dholmes-ora,
>> 
>> I checked what was happening.
>> 
>> The reproducer from JDK-8277444 simplified:
>> - Thread 1 does:
>> 
>> bigClass = Class.forName();
>> consumer.accept(bigClass); // Puts bigClass in QUEUE
>> final Object o = bigClass.getConstructor().newInstance();
>> System.out.println(o.hashCode());
>> 
>> 
>> - Thread 2 does:
>> 
>> final Class<?> aClass = QUEUE.take();
>> Instrumentation.retransformClasses(aClass); 
>> 
>> 
>> `Class.forName` does not link `bigClass`. So an unlinked class is put in a queue. The linking process starts when we start using `bigClass`.
>> Thread 2 gets unlinked `bigClass` from the queue. It can request to retransform  before we start using it and the linking process starts.
>> 
>> So we can have the linking process and the retransforming process running in parallel. There is no synchronization between them. We get a race condition. `bigClass` is big enough to make the linking process running long.
>> 
>> I think `Class.forName` does not do linking intentionally, for performance reasons.
>> 
>> I hope I've got everything correctly from logs and sources.
>
> @eastig Thank you very much for that detailed analysis. An interesting scenario. I still find it somewhat suspect that we can transform an unlinked class and wonder if we should instead ensure it is linked first, rather than trying to coordinate the two pieces of code via the use of the init_lock. ?

@dholmes-ora 

According to 
- https://docs.oracle.com/javase/specs/jls/se21/html/jls-12.html#jls-12.3
- https://docs.oracle.com/javase/specs/jvms/se21/html/jvms-5.html#jvms-5.4

they allow flexibility in an implementation of the linking process. If I am correct,  we have a "lazy" linkage strategy in Hotspot. I don't think we want to change anything here. 

`copy_bytecodes` is only used by JVMTI:
- `JvmtiEnv::RetransformClasses`
- `JvmtiEnv::GetBytecodes`

> I still find it somewhat suspect that we can transform an unlinked class and wonder if we should instead ensure it is linked first,

`JvmtiEnv::RetransformClasses` does not need a class to be linked. It needs the initial class file bytes which are the bytes passed to `ClassLoader.defineClass` or `RedefineClasses`. 
`JvmtiEnv::GetBytecodes` returns the bytecodes that implement the method. It's not clear from the JVMTI specification whether the returned bytecodes must be the initial class bytes. The current implementation returns the initial bytecodes the same used in `JvmtiEnv::RetransformClasses`.

As we don't keep the initial bytecodes (do we?), `copy_bytecodes` cannot be called during linking, linking changes bytecodes. It can only be called for a class in the unlinked and linked states. When `copy_bytecodes` is called for a linked class, it restores bytecodes to the initial ones. Linking cannot be done whilst `copy_bytecodes` is working.

If we use `init_lock`, `copy_bytecodes` will never see a class in the linking state. Linking will never see `copy_bytecodes`. As linking is blocked while we are copying bytecodes, the linking time will increase. This might have negative performance impact. Retrasforming obsoletes an original version of a method, if a new version of the method is installed. So we might not notice the performance impact.

What other options do we have which don't require many changes? We have two mutually exclusive processes.

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

PR Comment: https://git.openjdk.org/jdk/pull/26863#issuecomment-3214952273