Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Wisp] Fix potential issues: unbalanced monitors and killed arrayoop pending_unpark. #163

Merged
merged 1 commit into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3217,6 +3217,11 @@ void create_switchTo_contents(MacroAssembler *masm, int start, OopMapSet* oop_ma
__ strw(temp, Address(old_coroutine, Coroutine::thread_status_offset()));
__ ldrw(temp, Address(thread, JavaThread::java_call_counter_offset()));
__ strw(temp, Address(old_coroutine, Coroutine::java_call_counter_offset()));
__ ldr(temp, Address(thread, JavaThread::monitor_chunks_offset()));
__ str(temp, Address(old_coroutine, Coroutine::monitor_chunks_offset()));
__ ldrb(temp, Address(thread, JavaThread::do_not_unlock_if_synchronized_offset()));
__ strb(temp, Address(old_coroutine, Coroutine::do_not_unlock_if_synchronized_offset()));

__ mov(temp, sp);
__ ldr(old_stack, Address(old_coroutine, Coroutine::stack_offset()));
__ str(temp, Address(old_stack, CoroutineStack::last_sp_offset())); // str cannot use sp as an argument
Expand Down Expand Up @@ -3270,6 +3275,10 @@ void create_switchTo_contents(MacroAssembler *masm, int start, OopMapSet* oop_ma
__ strw(temp2, Address(temp, java_lang_Thread::thread_status_offset()));
__ ldrw(temp, Address(target_coroutine, Coroutine::java_call_counter_offset()));
__ strw(temp, Address(thread, JavaThread::java_call_counter_offset()));
__ ldr(temp, Address(target_coroutine, Coroutine::monitor_chunks_offset()));
__ str(temp, Address(thread, JavaThread::monitor_chunks_offset()));
__ ldrb(temp, Address(target_coroutine, Coroutine::do_not_unlock_if_synchronized_offset()));
__ strb(temp, Address(thread, JavaThread::do_not_unlock_if_synchronized_offset()));
#ifdef ASSERT
__ str(zr, Address(target_coroutine, Coroutine::handle_area_offset()));
__ str(zr, Address(target_coroutine, Coroutine::resource_area_offset()));
Expand Down
8 changes: 8 additions & 0 deletions src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4099,6 +4099,10 @@ void create_switchTo_contents(MacroAssembler *masm, int start, OopMapSet* oop_ma
__ movl(Address(old_coroutine, Coroutine::thread_status_offset()), temp);
__ movl(temp, Address(thread, JavaThread::java_call_counter_offset()));
__ movl(Address(old_coroutine, Coroutine::java_call_counter_offset()), temp);
__ movptr(temp, Address(thread, JavaThread::monitor_chunks_offset()));
__ movptr(Address(old_coroutine, Coroutine::monitor_chunks_offset()), temp);
__ movbool(temp, Address(thread, JavaThread::do_not_unlock_if_synchronized_offset()));
__ movbool(Address(old_coroutine, Coroutine::do_not_unlock_if_synchronized_offset()), temp);

// store CorotineStack.
// store rsp into CorotineStack.
Expand Down Expand Up @@ -4152,6 +4156,10 @@ void create_switchTo_contents(MacroAssembler *masm, int start, OopMapSet* oop_ma
__ movl(Address(temp2, java_lang_Thread::thread_status_offset()), temp);
__ movl(temp, Address(target_coroutine, Coroutine::java_call_counter_offset()));
__ movl(Address(thread, JavaThread::java_call_counter_offset()), temp);
__ movptr(temp, Address(target_coroutine, Coroutine::monitor_chunks_offset()));
__ movptr(Address(thread, JavaThread::monitor_chunks_offset()), temp);
__ movbool(temp, Address(target_coroutine, Coroutine::do_not_unlock_if_synchronized_offset()));
__ movbool(Address(thread, JavaThread::do_not_unlock_if_synchronized_offset()), temp);
#ifdef ASSERT
__ movptr(Address(target_coroutine, Coroutine::handle_area_offset()), (intptr_t)NULL_WORD);
__ movptr(Address(target_coroutine, Coroutine::resource_area_offset()), (intptr_t)NULL_WORD);
Expand Down
13 changes: 11 additions & 2 deletions src/hotspot/share/runtime/coroutine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ Coroutine* Coroutine::create_thread_coroutine(JavaThread* thread, CoroutineStack
#if defined(_WINDOWS)
coro->_last_SEH = NULL;
#endif
coro->_monitor_chunks = NULL;
coro->_do_not_unlock_if_synchronized = false;
coro->_wisp_thread = UseWispMonitor ? new WispThread(coro) : NULL;
if (UseWispMonitor) {
coro->_wisp_thread->set_thread_state(_thread_in_vm);
Expand Down Expand Up @@ -168,6 +170,8 @@ Coroutine* Coroutine::create_coroutine(JavaThread* thread, CoroutineStack* stack
#if defined(_WINDOWS)
coro->_last_SEH = NULL;
#endif
coro->_monitor_chunks = NULL;
coro->_do_not_unlock_if_synchronized = false;
coro->_wisp_thread = UseWispMonitor ? new WispThread(coro) : NULL;
if (UseWispMonitor) {
coro->_wisp_thread->set_thread_state(_thread_in_vm);
Expand Down Expand Up @@ -312,6 +316,10 @@ void Coroutine::oops_do(OopClosure* f, CodeBlobClosure* cf) {
DEBUG_CORO_ONLY(tty->print_cr("collecting handle area %08x", _handle_area));
_handle_area->oops_do(f);
_active_handles->oops_do(f);
// Traverse the monitor chunks
for (MonitorChunk* chunk = _monitor_chunks; chunk != NULL; chunk = chunk->next()) {
chunk->oops_do(f);
}
}
if (_wisp_task != NULL) {
f->do_oop((oop*) &_wisp_engine);
Expand Down Expand Up @@ -880,6 +888,8 @@ void WispThread::unpark(int task_id, bool using_wisp_park, bool proxy_unpark, Pa
}

int WispThread::get_proxy_unpark(jintArray res) {
HandleMark hm(JavaThread::current());
typeArrayHandle a(JavaThread::current(), typeArrayOop(JNIHandles::resolve_non_null(res)));
// We need to hoist code of safepoint state out of MutexLocker to prevent safepoint deadlock problem
// See the same usage: SR_lock in `JavaThread::exit()`
ThreadBlockInVM tbivm(JavaThread::current());
Expand All @@ -890,8 +900,7 @@ int WispThread::get_proxy_unpark(jintArray res) {
// we need to use _no_safepoint_check_flag, which won't yield a safepoint.
Wisp_lock->wait_without_safepoint_check();
}
typeArrayOop a = typeArrayOop(JNIHandles::resolve_non_null(res));
if (a == NULL) {
if (a.is_null()) {
return 0;
}
int copy_cnt = a->length() < _proxy_unpark->length() ? a->length() : _proxy_unpark->length();
Expand Down
14 changes: 12 additions & 2 deletions src/hotspot/share/runtime/coroutine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,14 @@ class Coroutine: public CHeapObj<mtThread>, public DoublyLinkedList<Coroutine> {
JNIHandleBlock* _active_handles;
GrowableArray<Metadata*>* _metadata_handles;
JavaFrameAnchor _anchor;
MonitorChunk* _monitor_chunks;
JavaThreadStatus _thread_status;
int _enable_steal_count;
int _java_call_counter;
int _last_native_call_counter;
int _clinit_call_counter;
volatile int _native_call_counter;
bool _do_not_unlock_if_synchronized;

// work steal pool
WispResourceArea* _wisp_post_steal_resource_area;
Expand Down Expand Up @@ -238,6 +240,8 @@ class Coroutine: public CHeapObj<mtThread>, public DoublyLinkedList<Coroutine> {
static ByteSize last_handle_mark_offset() { return byte_offset_of(Coroutine, _last_handle_mark); }
static ByteSize active_handles_offset() { return byte_offset_of(Coroutine, _active_handles); }
static ByteSize metadata_handles_offset() { return byte_offset_of(Coroutine, _metadata_handles); }
static ByteSize monitor_chunks_offset() { return byte_offset_of(Coroutine, _monitor_chunks); }
static ByteSize do_not_unlock_if_synchronized_offset() { return byte_offset_of(Coroutine, _do_not_unlock_if_synchronized); }
static ByteSize last_Java_sp_offset() {
return byte_offset_of(Coroutine, _anchor) + JavaFrameAnchor::last_Java_sp_offset();
}
Expand Down Expand Up @@ -466,8 +470,14 @@ class WispThread: public JavaThread {

virtual bool is_lock_owned(address adr) const {
CoroutineStack* stack = _coroutine->stack();
return stack->stack_base() >= adr &&
adr > (stack->stack_base() - stack->stack_size());
if (stack->stack_base() > adr && adr >= (stack->stack_base() - stack->stack_size())) {
return true;
}

for (MonitorChunk* chunk = monitor_chunks(); chunk != NULL; chunk = chunk->next()) {
if (chunk->contains(adr)) return true;
}
return false;
}

// we must set ObjectWaiter members before node enqueued(observed by other threads)
Expand Down
8 changes: 8 additions & 0 deletions src/hotspot/share/runtime/objectMonitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1201,6 +1201,14 @@ void ObjectMonitor::exit(JavaThread* current, bool not_suspended) {
" is exiting an ObjectMonitor it does not own.", p2i(current));
lsh.print_cr("The imbalance is possibly caused by JNI locking.");
print_debug_style_on(&lsh);
if (UseWispMonitor) {
JavaThread *pt = ((WispThread*)current)->thread();
tty->print_cr("[Wisp] Fatal IMSX: this=%p, pt=%p, current_coroutine=%p, self=%p (stack: %p - %p), _owner=%p, _owner->coro=%p",
this, pt, ((JavaThread*)pt)->current_coroutine(), current,
((WispThread*)current)->coroutine()->stack()->stack_base(),
((WispThread*)current)->coroutine()->stack()->stack_base() - ((WispThread*)current)->coroutine()->stack()->stack_size(),
_owner, ((WispThread*)_owner)->coroutine());
}
assert(false, "Non-balanced monitor enter/exit!");
#endif
return;
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/runtime/thread.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,7 @@ class JavaThread: public Thread {
bool wisp_preempted() const { return _wisp_preempted; }
void set_wisp_preempted(bool b) { _wisp_preempted = b; }

static ByteSize monitor_chunks_offset() { return byte_offset_of(JavaThread, _monitor_chunks); }
static ByteSize current_coroutine_offset() { return byte_offset_of(JavaThread, _current_coroutine); }
void initialize_thread_coroutine();

Expand Down
Loading