Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8253694: Remove Thread::muxAcquire() from ThreadCrashProtection()
Reviewed-by: dholmes, dcubed, coleenp
  • Loading branch information
pchilano committed Oct 6, 2020
1 parent d2b1dc6 commit 57493c1
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 20 deletions.
10 changes: 2 additions & 8 deletions src/hotspot/os/posix/os_posix.cpp
Expand Up @@ -1525,9 +1525,10 @@ bool os::Posix::matches_effective_uid_and_gid_or_root(uid_t uid, gid_t gid) {

Thread* os::ThreadCrashProtection::_protected_thread = NULL;
os::ThreadCrashProtection* os::ThreadCrashProtection::_crash_protection = NULL;
volatile intptr_t os::ThreadCrashProtection::_crash_mux = 0;

os::ThreadCrashProtection::ThreadCrashProtection() {
_protected_thread = Thread::current();
assert(_protected_thread->is_JfrSampler_thread(), "should be JFRSampler");
}

/*
Expand All @@ -1539,11 +1540,6 @@ os::ThreadCrashProtection::ThreadCrashProtection() {
bool os::ThreadCrashProtection::call(os::CrashProtectionCallback& cb) {
sigset_t saved_sig_mask;

Thread::muxAcquire(&_crash_mux, "CrashProtection");

_protected_thread = Thread::current_or_null();
assert(_protected_thread != NULL, "Cannot crash protect a NULL thread");

// we cannot rely on sigsetjmp/siglongjmp to save/restore the signal mask
// since on at least some systems (OS X) siglongjmp will restore the mask
// for the process, not the thread
Expand All @@ -1556,14 +1552,12 @@ bool os::ThreadCrashProtection::call(os::CrashProtectionCallback& cb) {
// and clear the crash protection
_crash_protection = NULL;
_protected_thread = NULL;
Thread::muxRelease(&_crash_mux);
return true;
}
// this happens when we siglongjmp() back
pthread_sigmask(SIG_SETMASK, &saved_sig_mask, NULL);
_crash_protection = NULL;
_protected_thread = NULL;
Thread::muxRelease(&_crash_mux);
return false;
}

Expand Down
3 changes: 1 addition & 2 deletions src/hotspot/os/posix/os_posix.hpp
Expand Up @@ -146,7 +146,7 @@ class Posix {
};

/*
* Crash protection for the watcher thread. Wrap the callback
* Crash protection for the JfrSampler thread. Wrap the callback
* with a sigsetjmp and in case of a SIGSEGV/SIGBUS we siglongjmp
* back.
* To be able to use this - don't take locks, don't rely on destructors,
Expand All @@ -167,7 +167,6 @@ class ThreadCrashProtection : public StackObj {
private:
static Thread* _protected_thread;
static ThreadCrashProtection* _crash_protection;
static volatile intptr_t _crash_mux;
void restore();
sigjmp_buf _jmpbuf;
};
Expand Down
10 changes: 2 additions & 8 deletions src/hotspot/os/windows/os_windows.cpp
Expand Up @@ -5066,9 +5066,10 @@ void os::pause() {

Thread* os::ThreadCrashProtection::_protected_thread = NULL;
os::ThreadCrashProtection* os::ThreadCrashProtection::_crash_protection = NULL;
volatile intptr_t os::ThreadCrashProtection::_crash_mux = 0;

os::ThreadCrashProtection::ThreadCrashProtection() {
_protected_thread = Thread::current();
assert(_protected_thread->is_JfrSampler_thread(), "should be JFRSampler");
}

// See the caveats for this class in os_windows.hpp
Expand All @@ -5078,12 +5079,6 @@ os::ThreadCrashProtection::ThreadCrashProtection() {
// The callback is supposed to provide the method that should be protected.
//
bool os::ThreadCrashProtection::call(os::CrashProtectionCallback& cb) {

Thread::muxAcquire(&_crash_mux, "CrashProtection");

_protected_thread = Thread::current_or_null();
assert(_protected_thread != NULL, "Cannot crash protect a NULL thread");

bool success = true;
__try {
_crash_protection = this;
Expand All @@ -5094,7 +5089,6 @@ bool os::ThreadCrashProtection::call(os::CrashProtectionCallback& cb) {
}
_crash_protection = NULL;
_protected_thread = NULL;
Thread::muxRelease(&_crash_mux);
return success;
}

Expand Down
3 changes: 1 addition & 2 deletions src/hotspot/os/windows/os_windows.hpp
Expand Up @@ -128,7 +128,7 @@ class win32 {
};

/*
* Crash protection for the watcher thread. Wrap the callback
* Crash protection for the JfrSampler thread. Wrap the callback
* with a __try { call() }
* To be able to use this - don't take locks, don't rely on destructors,
* don't make OS library calls, don't allocate memory, don't print,
Expand All @@ -146,7 +146,6 @@ class ThreadCrashProtection : public StackObj {
private:
static Thread* _protected_thread;
static ThreadCrashProtection* _crash_protection;
static volatile intptr_t _crash_mux;
};

class PlatformEvent : public CHeapObj<mtSynchronizer> {
Expand Down
Expand Up @@ -343,6 +343,7 @@ class JfrThreadSampler : public NonJavaThread {
virtual void post_run();
public:
virtual char* name() const { return (char*)"JFR Thread Sampler"; }
bool is_JfrSampler_thread() const { return true; }
void run();
static Monitor* transition_block() { return JfrThreadSampler_lock; }
static void on_javathread_suspend(JavaThread* thread);
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/runtime/thread.hpp
Expand Up @@ -490,6 +490,7 @@ class Thread: public ThreadShadow {
virtual bool is_ConcurrentGC_thread() const { return false; }
virtual bool is_Named_thread() const { return false; }
virtual bool is_Worker_thread() const { return false; }
virtual bool is_JfrSampler_thread() const { return false; }

// Can this thread make Java upcalls
virtual bool can_call_java() const { return false; }
Expand Down

1 comment on commit 57493c1

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented on 57493c1 Oct 6, 2020

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.