Skip to content

Commit

Permalink
[Wisp] Fix coroutine miss nmethod marking cause by work stealing.
Browse files Browse the repository at this point in the history
Summary:
NMethodMarkingClosure will do handshake to scan nmethod and mark out of safepoint. If a coroutine is steal from a not scanned thread to a scanned thread. The coroutine will miss current round of nmethod marking then collected by mistake. The above case will lead to a crash.

Test Plan: all wisp tests

Reviewed-by: yulei

Issue:
#176
  • Loading branch information
nebula-xm authored and yuleil committed Nov 16, 2023
1 parent 81f8572 commit 717f638
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/hotspot/share/prims/unsafe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,11 @@ JVM_ENTRY(jboolean, CoroutineSupport_stealCoroutine(JNIEnv* env, jclass klass, j
assert(coro->state() != Coroutine::_current, "running");

CoroutineListLocker cll(coro->thread(), thread);
// check if the source thread and the target thread has been scanned or not
// only if hold the CoroutineListLocker, and has the same scanned state, then we can steal
if (coro->thread()->nmethod_traversals() != thread->nmethod_traversals()) {
return false;
}
coro->remove_from_list(coro->thread()->coroutine_list());
coro->insert_into_list(thread->coroutine_list());
// change thread logic
Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/runtime/sweeper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ class NMethodMarkingClosure : public HandshakeClosure {
void do_thread(Thread* thread) {
if (thread->is_Java_thread() && ! thread->is_Code_cache_sweeper_thread()) {
thread->as_Java_thread()->nmethods_do(_cl);
if (EnableCoroutine) {
thread->as_Java_thread()->set_nmethod_traversals(NMethodSweeper::traversal_count());
}
}
}
};
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/runtime/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,7 @@ JavaThread::JavaThread() :
_coroutine_list(nullptr),
_current_coroutine(nullptr),
_wisp_preempted(false),
_nmethod_traversals(0),

_handshake(this),

Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/runtime/thread.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1047,6 +1047,7 @@ class JavaThread: public Thread {
Coroutine* _coroutine_list;
Coroutine* _current_coroutine;
bool _wisp_preempted;
volatile long _nmethod_traversals;

public:
volatile int* const coroutine_list_lock() { return &_coroutine_list_lock; }
Expand All @@ -1055,6 +1056,8 @@ class JavaThread: public Thread {
void set_current_coroutine(Coroutine *coro) { _current_coroutine = coro; }
bool wisp_preempted() const { return _wisp_preempted; }
void set_wisp_preempted(bool b) { _wisp_preempted = b; }
void set_nmethod_traversals(long n) { Atomic::release_store(&_nmethod_traversals, n); }
long nmethod_traversals() const { return Atomic::load_acquire(&_nmethod_traversals); }

static ByteSize monitor_chunks_offset() { return byte_offset_of(JavaThread, _monitor_chunks); }
static ByteSize current_coroutine_offset() { return byte_offset_of(JavaThread, _current_coroutine); }
Expand Down

0 comments on commit 717f638

Please sign in to comment.