Skip to content

Commit

Permalink
8271348: Add stronger sanity check of thread state when polling for s…
Browse files Browse the repository at this point in the history
…afepoint/handshakes

Co-authored-by: Patricio Chilano Mateo <pchilanomate@openjdk.org>
Reviewed-by: dholmes, pchilanomate
  • Loading branch information
Daniel D. Daugherty and pchilano committed Aug 2, 2021
1 parent 3e3051e commit db950ca
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 28 deletions.
45 changes: 17 additions & 28 deletions src/hotspot/share/runtime/safepoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -705,45 +705,34 @@ void SafepointSynchronize::block(JavaThread *thread) {
}

JavaThreadState state = thread->thread_state();
assert(is_a_block_safe_state(state), "Illegal threadstate encountered: %d", state);
thread->frame_anchor()->make_walkable(thread);

uint64_t safepoint_id = SafepointSynchronize::safepoint_counter();
// Check that we have a valid thread_state at this point
switch(state) {
case _thread_in_vm_trans:
case _thread_in_Java: // From compiled code
case _thread_in_native_trans:
case _thread_blocked_trans:
case _thread_new_trans:

// We have no idea where the VMThread is, it might even be at next safepoint.
// So we can miss this poll, but stop at next.

// Load dependent store, it must not pass loading of safepoint_id.
thread->safepoint_state()->set_safepoint_id(safepoint_id); // Release store
// We have no idea where the VMThread is, it might even be at next safepoint.
// So we can miss this poll, but stop at next.

// This part we can skip if we notice we miss or are in a future safepoint.
OrderAccess::storestore();
// Load in wait barrier should not float up
thread->set_thread_state_fence(_thread_blocked);
// Load dependent store, it must not pass loading of safepoint_id.
thread->safepoint_state()->set_safepoint_id(safepoint_id); // Release store

_wait_barrier->wait(static_cast<int>(safepoint_id));
assert(_state != _synchronized, "Can't be");
// This part we can skip if we notice we miss or are in a future safepoint.
OrderAccess::storestore();
// Load in wait barrier should not float up
thread->set_thread_state_fence(_thread_blocked);

// If barrier is disarmed stop store from floating above loads in barrier.
OrderAccess::loadstore();
thread->set_thread_state(state);
_wait_barrier->wait(static_cast<int>(safepoint_id));
assert(_state != _synchronized, "Can't be");

// Then we reset the safepoint id to inactive.
thread->safepoint_state()->reset_safepoint_id(); // Release store
// If barrier is disarmed stop store from floating above loads in barrier.
OrderAccess::loadstore();
thread->set_thread_state(state);

OrderAccess::fence();
// Then we reset the safepoint id to inactive.
thread->safepoint_state()->reset_safepoint_id(); // Release store

break;
OrderAccess::fence();

default:
fatal("Illegal threadstate encountered: %d", state);
}
guarantee(thread->safepoint_state()->get_safepoint_id() == InactiveSafepointCounter,
"The safepoint id should be set only in block path");

Expand Down
13 changes: 13 additions & 0 deletions src/hotspot/share/runtime/safepoint.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,19 @@ class SafepointSynchronize : AllStatic {
JavaThread *thread,
uint64_t safepoint_count);

static bool is_a_block_safe_state(JavaThreadState state) {
// Check that we have a valid thread_state before blocking for safepoints
switch(state) {
case _thread_in_vm_trans:
case _thread_in_Java: // From compiled code
case _thread_in_native_trans:
case _thread_blocked_trans:
case _thread_new_trans:
return true;
default:
return false;
}
}
// Called when a thread voluntarily blocks
static void block(JavaThread *thread);

Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/runtime/safepointMechanism.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ void SafepointMechanism::process(JavaThread *thread, bool allow_suspend) {
// local poll already checked, if used.
bool need_rechecking;
do {
JavaThreadState state = thread->thread_state();
guarantee(SafepointSynchronize::is_a_block_safe_state(state), "Illegal threadstate encountered: %d", state);
if (global_poll()) {
// Any load in ::block() must not pass the global poll load.
// Otherwise we might load an old safepoint counter (for example).
Expand Down

1 comment on commit db950ca

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.