Greetings,

For this one liner review, I'd love to hear from David H., Thomas Stüfe,
and Serguei Spitsyn.

David Holmes spotted a locking problem with my fix for the following bug:

    JDK-8205195 NestedThreadsListHandleInErrorHandlingTest fails because
                hs_err doesn't contain _nested_thread_list_max
    https://bugs.openjdk.java.net/browse/JDK-8205195

    Webrev URL: http://cr.openjdk.java.net/~dcubed/8205195-webrev/0-for-jdk-jdk/

The fix for the issue is a one liner that only grabs the Threads_lock
when the caller does not already own it:

$ hg diff
diff -r 5698cf4e50f1 src/hotspot/share/utilities/vmError.cpp
--- a/src/hotspot/share/utilities/vmError.cpp Fri Jun 22 12:15:16 2018 -0400
+++ b/src/hotspot/share/utilities/vmError.cpp Mon Jun 25 21:20:52 2018 -0400
@@ -1703,7 +1703,7 @@
   // from racing with Threads::add() or Threads::remove() as we
   // generate the hs_err_pid file. This makes our ErrorHandling tests
   // more stable.
- MutexLockerEx ml(Threads_lock, Mutex::_no_safepoint_check_flag);
+ MutexLockerEx ml(Threads_lock->owned_by_self() ? NULL : Threads_lock, Mutex::_no_safepoint_check_flag);

   switch (how) {
     case 1: vmassert(str == NULL, "expected null"); break;


Figuring out a way to detect the failure mode in order to test the
fix was much more involved than the fix itself. Gory details are
in the bug:

    JDK-8205648 fix for 8205195 breaks secondary error handling
    https://bugs.openjdk.java.net/browse/JDK-8205648

No webrev since the one liner diff is shown above. This fix was tested
with the test/hotspot/jtreg/runtime/ErrorHandling tests on Linux-X64
and with a special version of ErrorHandling/SecondaryErrorTest.java
that is documented in JDK-8205648.

Thanks, in advance, for any comments, questions or suggestions.

Dan

Reply via email to