Skip to content

Commit

Permalink
[Wisp] Fix potential issues: unbalanced monitors and killed arrayoop …
Browse files Browse the repository at this point in the history
…pending_unpark.

Summary: As the title.

Test Plan: test/jdk/com/alibaba/wisp

Reviewed-by: yulei

Issue:
dragonwell-project#162
  • Loading branch information
ZhaiMo15 authored and yuleil committed Oct 13, 2023
1 parent 65f60d6 commit c6bca3c
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 4 deletions.
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

0 comments on commit c6bca3c

Please sign in to comment.