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

[Daniel D. Daugherty]

Thanks! And thanks for catching the bug itself.

Dan


On 6/26/18 11:43 PM, David Holmes wrote:
> 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