On Fri, 29 Aug 2025 22:48:06 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 linking a class before the `copy_bytecodes` method is called.
>> The PR also adds a regression test.
>>
>> Tested fastdebug and release builds: Linux x86_64 and arm64
>> - The reproducer from JDK-8277444 passed.
>> - The regression test passed.
>> - Tier1 - tier3 passed.
>
> Evgeny Astigeevich 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 five additional
> commits since the last revision:
>
> - Merge branch 'master' into JDK-8277444
> - Link classes before copy_bytecodes; Add regression test
> - Symplify comments; Get JavaThread::current in variable
> - Add missing include runtime/synchronizer.hpp
> - 8277444: Race condition on Instrumentation.retransformClasses() and class
> linking
I had a couple of minor comments but otherwise looks good. Is the test now
reliable? Thank you for adding a test.
src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp line 35:
> 33: #include "runtime/handles.inline.hpp"
> 34: #include "runtime/signature.hpp"
> 35: #include "runtime/synchronizer.hpp"
You don't need this include anymore.
src/hotspot/share/prims/jvmtiEnv.cpp line 3446:
> 3444: current_thread->clear_pending_exception();
> 3445: return JVMTI_ERROR_INVALID_CLASS;
> 3446: }
Can you use the pattern:
JavaThread* THREAD = current_thread;
... link_class(THREAD);
if (HAS_PENDING_EXCEPTION)
etc.
-------------
PR Review: https://git.openjdk.org/jdk/pull/26863#pullrequestreview-3176231981
PR Review Comment: https://git.openjdk.org/jdk/pull/26863#discussion_r2315873660
PR Review Comment: https://git.openjdk.org/jdk/pull/26863#discussion_r2315880585
