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
8263191: Consolidate ThreadInVMfromJavaNoAsyncException and ThreadBlockInVMWithDeadlockCheck with existing wrappers #2880
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -150,17 +150,19 @@ class ThreadInVMForHandshake : public ThreadStateTransition { | |
}; | ||
|
||
class ThreadInVMfromJava : public ThreadStateTransition { | ||
bool _check_asyncs; | ||
public: | ||
ThreadInVMfromJava(JavaThread* thread) : ThreadStateTransition(thread) { | ||
ThreadInVMfromJava(JavaThread* thread, bool check_asyncs = true) : ThreadStateTransition(thread), _check_asyncs(check_asyncs) { | ||
trans_from_java(_thread_in_vm); | ||
} | ||
~ThreadInVMfromJava() { | ||
if (_thread->stack_overflow_state()->stack_yellow_reserved_zone_disabled()) { | ||
_thread->stack_overflow_state()->enable_stack_yellow_reserved_zone(); | ||
} | ||
trans(_thread_in_vm, _thread_in_Java); | ||
// Check for pending. async. exceptions or suspends. | ||
if (_thread->has_special_runtime_exit_condition()) _thread->handle_special_runtime_exit_condition(); | ||
// We prevent asynchronous exceptions from being installed on return to Java in situations | ||
// where we can't tolerate them. See bugs: 4324348, 4854693, 4998314, 5040492, 5050705. | ||
if (_thread->has_special_runtime_exit_condition()) _thread->handle_special_runtime_exit_condition(_check_asyncs); | ||
} | ||
}; | ||
|
||
|
@@ -222,93 +224,45 @@ class ThreadToNativeFromVM : public ThreadStateTransition { | |
}; | ||
|
||
|
||
// Parameter in_flight_mutex_addr is only used by class Mutex to avoid certain deadlock | ||
// scenarios while making transitions that might block for a safepoint or handshake. | ||
// It's the address of a pointer to the mutex we are trying to acquire. This will be used to | ||
// access and release said mutex when transitioning back from blocked to vm (destructor) in | ||
// case we need to stop for a safepoint or handshake. | ||
class ThreadBlockInVM : public ThreadStateTransition { | ||
public: | ||
ThreadBlockInVM(JavaThread *thread) | ||
: ThreadStateTransition(thread) { | ||
// Once we are blocked vm expects stack to be walkable | ||
thread->frame_anchor()->make_walkable(thread); | ||
trans(_thread_in_vm, _thread_blocked); | ||
} | ||
~ThreadBlockInVM() { | ||
trans(_thread_blocked, _thread_in_vm); | ||
// We don't need to clear_walkable because it will happen automagically when we return to java | ||
} | ||
}; | ||
|
||
// Unlike ThreadBlockInVM, this class is designed to avoid certain deadlock scenarios while making | ||
// transitions inside class Mutex in cases where we need to block for a safepoint or handshake. It | ||
// receives an extra argument compared to ThreadBlockInVM, the address of a pointer to the mutex we | ||
// are trying to acquire. This will be used to access and release the mutex if needed to avoid | ||
// said deadlocks. | ||
// It works like ThreadBlockInVM but differs from it in two ways: | ||
// - When transitioning in (constructor), it checks for safepoints without blocking, i.e., calls | ||
// back if needed to allow a pending safepoint to continue but does not block in it. | ||
// - When transitioning back (destructor), if there is a pending safepoint or handshake it releases | ||
// the mutex that is only partially acquired. | ||
class ThreadBlockInVMWithDeadlockCheck : public ThreadStateTransition { | ||
private: | ||
Mutex** _in_flight_mutex_addr; | ||
|
||
void release_mutex() { | ||
assert(_in_flight_mutex_addr != NULL, "_in_flight_mutex_addr should have been set on constructor"); | ||
Mutex* in_flight_mutex = *_in_flight_mutex_addr; | ||
if (in_flight_mutex != NULL) { | ||
in_flight_mutex->release_for_safepoint(); | ||
*_in_flight_mutex_addr = NULL; | ||
} | ||
} | ||
public: | ||
ThreadBlockInVMWithDeadlockCheck(JavaThread* thread, Mutex** in_flight_mutex_addr) | ||
ThreadBlockInVM(JavaThread* thread, Mutex** in_flight_mutex_addr = NULL) | ||
: ThreadStateTransition(thread), _in_flight_mutex_addr(in_flight_mutex_addr) { | ||
assert(thread->thread_state() == _thread_in_vm, "coming from wrong thread state"); | ||
thread->check_possible_safepoint(); | ||
// Once we are blocked vm expects stack to be walkable | ||
thread->frame_anchor()->make_walkable(thread); | ||
|
||
// All unsafe states are treated the same by the VMThread | ||
// so we can skip the _thread_in_vm_trans state here. Since | ||
// we don't read poll, it's enough to order the stores. | ||
OrderAccess::storestore(); | ||
|
||
thread->set_thread_state(_thread_blocked); | ||
Comment on lines
-267
to
243
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're going to set the thread_state directly here, I think you need |
||
} | ||
~ThreadBlockInVMWithDeadlockCheck() { | ||
~ThreadBlockInVM() { | ||
assert(_thread->thread_state() == _thread_blocked, "coming from wrong thread state"); | ||
// Change to transition state and ensure it is seen by the VM thread. | ||
_thread->set_thread_state_fence((JavaThreadState)(_thread_blocked_trans)); | ||
_thread->set_thread_state_fence(_thread_blocked_trans); | ||
|
||
if (SafepointMechanism::should_process(_thread)) { | ||
release_mutex(); | ||
if (_in_flight_mutex_addr != NULL) { | ||
release_mutex(); | ||
} | ||
SafepointMechanism::process_if_requested(_thread); | ||
} | ||
|
||
_thread->set_thread_state(_thread_in_vm); | ||
} | ||
}; | ||
|
||
|
||
// This special transition class is only used to prevent asynchronous exceptions | ||
// from being installed on vm exit in situations where we can't tolerate them. | ||
// See bugs: 4324348, 4854693, 4998314, 5040492, 5050705. | ||
class ThreadInVMfromJavaNoAsyncException : public ThreadStateTransition { | ||
public: | ||
ThreadInVMfromJavaNoAsyncException(JavaThread* thread) : ThreadStateTransition(thread) { | ||
trans_from_java(_thread_in_vm); | ||
} | ||
~ThreadInVMfromJavaNoAsyncException() { | ||
if (_thread->stack_overflow_state()->stack_yellow_reserved_zone_disabled()) { | ||
_thread->stack_overflow_state()->enable_stack_yellow_reserved_zone(); | ||
void release_mutex() { | ||
Mutex* in_flight_mutex = *_in_flight_mutex_addr; | ||
if (in_flight_mutex != NULL) { | ||
in_flight_mutex->release_for_safepoint(); | ||
*_in_flight_mutex_addr = NULL; | ||
} | ||
trans(_thread_in_vm, _thread_in_Java); | ||
// NOTE: We do not check for pending. async. exceptions. | ||
// If we did and moved the pending async exception over into the | ||
// pending exception field, we would need to deopt (currently C2 | ||
// only). However, to do so would require that we transition back | ||
// to the _thread_in_vm state. Instead we postpone the handling of | ||
// the async exception. | ||
|
||
|
||
// Check for pending. suspends only. | ||
if (_thread->has_special_runtime_exit_condition()) | ||
_thread->handle_special_runtime_exit_condition(false); | ||
} | ||
}; | ||
|
||
|
@@ -383,7 +337,7 @@ class VMNativeEntryWrapper { | |
|
||
#define JRT_ENTRY_NO_ASYNC(result_type, header) \ | ||
result_type header { \ | ||
ThreadInVMfromJavaNoAsyncException __tiv(thread); \ | ||
ThreadInVMfromJava __tiv(thread, false /* check asyncs */); \ | ||
VM_ENTRY_BASE(result_type, header, thread) \ | ||
debug_only(VMEntryWrapper __vew;) | ||
|
||
|
@@ -401,7 +355,7 @@ class VMNativeEntryWrapper { | |
|
||
#define JRT_BLOCK_NO_ASYNC \ | ||
{ \ | ||
ThreadInVMfromJavaNoAsyncException __tiv(thread); \ | ||
ThreadInVMfromJava __tiv(thread, false /* check asyncs */); \ | ||
Thread* THREAD = thread; \ | ||
debug_only(VMEntryWrapper __vew;) | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new ThreadBlockInVM is not equivalent to the old one.
The old one used
trans(_thread_in_vm, _thread_blocked)
whichasserted that the "from" state is equal to
_thread_in_vm
. Thisnew code does not do that. The trans() call also resulted in a
SafepointMechanism::process_if_requested(thread);
call afterwe transitioned the thread to
_thread_in_vm
+1 and before weset the new thread state to
_thread_blocked
.