RFR: 8369238: Allow virtual thread preemption on some common class initialization paths [v3]

[Patricio Chilano Mateo]

On Fri, 17 Oct 2025 06:49:39 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Patricio Chilano Mateo has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix verify_frame_kind
>
> src/hotspot/share/interpreter/linkResolver.cpp line 1122:
> 
>> 1120:       resolved_klass->initialize(CHECK);
>> 1121:     } else if (static_mode == StaticMode::initialize_klass_preemptable) {
>> 1122:       resolved_klass->initialize_preemptable(CHECK);
> 
> Why is this not CHECK_AND_CLEAR_PREEMPTED?

We need to let the exception propagate all the way back to the VM entry point, and only then we can clear it.

> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 196:
> 
>> 194: static void do_deopt_after_thaw(JavaThread* thread);
>> 195: static bool do_verify_after_thaw(JavaThread* thread, stackChunkOop chunk, outputStream* st);
>> 196: static void log_frames(JavaThread* thread, bool dolog = true);
> 
> Suggestion:
> 
> static void log_frames(JavaThread* thread, bool do_log = true);

I removed it altogether. I used it during debugging when I wanted to disable all frame logging except from certain places. But it's not really needed.

> src/hotspot/share/runtime/javaThread.hpp line 492:
> 
>> 490:   // throw IE at the end of thawing before returning to Java.
>> 491:   bool _pending_interrupted_exception;
>> 492:   // We allow preemption on some klass initializion calls.
> 
> Suggestion:
> 
>   // We allow preemption on some klass initialization calls.

Fixed.

> src/hotspot/share/runtime/javaThread.hpp line 507:
> 
>> 505: 
>> 506:   bool at_preemptable_init() { return _at_preemptable_init; }
>> 507:   void set_at_preemptable_init(bool b) { _at_preemptable_init = b; }
> 
> If you are going align 503 and 504 then you may as well align these two lines as well

Fixed.

> src/hotspot/share/runtime/javaThread.hpp line 522:
> 
>> 520:     JavaThread* _thread;
>> 521:    public:
>> 522:     AtRedoVMCall(JavaThread *t) : _thread(t) {
> 
> Suggestion:
> 
>     AtRedoVMCall(JavaThread* t) : _thread(t) {

Fixed.

> src/hotspot/share/runtime/javaThread.hpp line 524:
> 
>> 522:     AtRedoVMCall(JavaThread *t) : _thread(t) {
>> 523:       _thread->_interp_at_preemptable_vmcall_cnt++;
>> 524:       assert(_thread->_interp_at_preemptable_vmcall_cnt > 0, "");
> 
> Suggestion:
> 
>       assert(_thread->_interp_at_preemptable_vmcall_cnt > 0, "Unexpected count: %d", _thread->_interp_at_preemptable_vmcall_cnt);

Added.

> src/hotspot/share/runtime/javaThread.hpp line 528:
> 
>> 526:     ~AtRedoVMCall() {
>> 527:       _thread->_interp_at_preemptable_vmcall_cnt--;
>> 528:       assert(_thread->_interp_at_preemptable_vmcall_cnt >= 0, "");
> 
> Suggestion:
> 
>     assert(_thread->_interp_at_preemptable_vmcall_cnt > 0, "Unexpected count: %d", _thread->_interp_at_preemptable_vmcall_cnt);

Added.

> src/hotspot/share/runtime/objectMonitor.cpp line 315:
> 
>> 313: }
>> 314: 
>> 315: // Keep object protected during ObjectLocker preemption.
> 
> I don't understand why this is necessary.

Once the thread initializing the class finishes the initialization and calls `InstanceKlass::fence_and_clear_init_lock()`, the init_lock oop can be GC’ed, unless there is some other `Handle` protecting it (`ObjectMonitor::_object` is only a `WeakHandle`). But we could still have preempted virtual threads using the ObjectMonitor, so we need to protect the oop. In practice, I don’t see paths in `ObjectMonitor::resume_operation` where we try to get the oop out of the monitor, but I’m not sure if we want to break the invariant where the object needs to be alive while using its ObjectMonitor.

> src/hotspot/share/runtime/synchronizer.cpp line 489:
> 
>> 487:     if (_thread->preempting()) {
>> 488:       // If preemption was cancelled we acquired the monitor after freezing
>> 489:       // the frames. Redoing the vm call later in thaw will require us to
> 
> I don't quite follow the control flow here - at what point have we frozen any frames? Was that deep inside `enter`?

Yes, in `ObjectMonitor::enter_with_contention_mark` we first freezed the frames, but then the thread was able to acquire the monitor in `ObjectMonitor::vthread_monitor_enter` while trying to add itself to the `_entry_list`, so preemption was cancelled. For this `ObjectLocker` case, we will have to redo the VM call again after thawing so we need to release the monitor.

> src/java.base/share/classes/java/lang/VirtualThread.java line 172:
> 
>> 170: 
>> 171:     // true when waiting in Object.wait, false for VM internal uninterruptible Object.wait
>> 172:     private volatile boolean interruptableWait;
> 
> Suggestion:
> 
>     private volatile boolean interruptibleWait;

Fixed.

> src/java.base/share/classes/java/lang/VirtualThread.java line 605:
> 
>> 603:         if (s == WAITING || s == TIMED_WAITING) {
>> 604:             int newState;
>> 605:             boolean interruptable = interruptableWait;
> 
> Suggestion:
> 
>             boolean interruptible = interruptibleWait;

Fixed.

> src/java.base/share/classes/jdk/internal/vm/PreemptedException.java line 31:
> 
>> 29:  * Internal exception used only by the VM.
>> 30:  */
>> 31: public class PreemptedException extends Exception {
> 
> Should probably extend `RuntimeException` so that it is not considered a checked-exception

Changed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445892049
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445884886
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445886314
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445886550
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445886740
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445886915
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445887072
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445889274
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445890688
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445890950
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445891154
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2445891449