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

[David Holmes]

Looks good Dan!

Thanks,
David

On 26/06/2018 11:06 PM, Daniel D. Daugherty wrote:
> 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