RFR(XXXS): 8205648 fix for 8205195 breaks secondary error handling

Daniel D. Daugherty daniel.daugherty at oracle.com
Tue Jun 26 13:06:27 UTC 2018


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


More information about the hotspot-runtime-dev mailing list