On Wed, 20 Aug 2025 17:31:22 GMT, Evgeny Astigeevich <[email protected]> 
wrote:

>> There is a race between `JvmtiClassFileReconstituter::copy_bytecodes` and 
>> `InstanceKlass::link_class_impl`.  `InstanceKlass::link_class_impl` can be 
>> rewriting bytecodes. `JvmtiClassFileReconstituter::copy_bytecodes` will not 
>> restore them to the original ones because the flag `rewritten` is `false`. 
>> This will result in invalid bytecode.
>> 
>> This PR adds a lock (`init_lock`) to the `copy_bytecodes` method to prevent 
>> reading bytecodes while they are being rewritten during class linking.
>> 
>> Tested fastdebug and release builds: Linux x86_64 and arm64
>> - The reproducer from JDK-8277444 passed.
>> - Tier1 - tier3 passed.
>
> Evgeny Astigeevich has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add missing include runtime/synchronizer.hpp

I've heard from Coleen and she seems okay with this approach; and Patricio 
doesn't think it will have an impact on the virtual thread work - so that eases 
my concerns. I have some minor requested changes whilst we wait for a 
serviceability review.

Thanks

src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp line 1006:

> 1004:   // at the same time the linking process are rewriting them.
> 1005:   Handle h_init_lock(Thread::current(), 
> mh->method_holder()->init_lock());
> 1006:   ObjectLocker ol(h_init_lock, JavaThread::current());

Suggestion:

  JavaThread* current = JavaThread::current();
  Handle h_init_lock(current, mh->method_holder()->init_lock());
  ObjectLocker ol(h_init_lock, current);

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26863#pullrequestreview-3152910901
PR Review Comment: https://git.openjdk.org/jdk/pull/26863#discussion_r2299048252

Reply via email to