RFR: 8253694: Remove Thread::muxAcquire() from ThreadCrashProtection() [v2]

Daniel D.Daugherty dcubed at openjdk.java.net
Mon Oct 5 16:04:49 UTC 2020


On Mon, 5 Oct 2020 14:12:55 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> Hi all,
>> 
>> Please review the following patch. Current ThreadCrashProtection() implementation uses static members which requires
>> the use of Thread::muxAcquire() to allow only one user at a time. We can avoid this synchronization requirement if each
>> thread has its own ThreadCrashProtection *data. I tested it builds on Linux, macOS and Windows. Since the
>> JfrThreadSampler is the only one using this I run all the tests from test/jdk/jdk/jfr/. I also run some tests with JFR
>> enabled while forcing a crash in OSThreadSampler::protected_task() and tests passed with several "Thread method sampler
>> crashed" UL output. Also run tiers1-3.  Thanks, Patricio
>
> Patricio Chilano Mateo has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove _crash_mux

Looks good. I only have minor suggested changes, but it's you call
on whether to make those changes.

src/hotspot/os/posix/os_posix.cpp line 1530:

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

Perhaps this:
  `_protected_thread = Thread::current();`
  `assert(_protected_thread->is_JfrSampler_thread(), "should be JFRSampler");`
would be a little more clean.

src/hotspot/os/windows/os_windows.cpp line 5005:

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

Perhaps this:
  `_protected_thread = Thread::current();`
  `assert(_protected_thread->is_JfrSampler_thread(), "should be JFRSampler");`
would be a little more clean.

-------------

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/376


More information about the hotspot-jfr-dev mailing list