RFR(XXS): 8077392 and 8131715 - fix for hang in Contended Locking "fast enter" bucket

[Daniel D. Daugherty]

On 4/1/16 3:23 PM, Karen Kinnear wrote:
> Dan,
>
> Impressive debugging and analysis!

Thanks! It was a long hunt, but worth it...


> Thank you for the detailed notes.

You're very welcome! Hope you enjoyed the nap... :-)


> Would you mind adding to the comment that for performance, stack walkers check biased_locking first, then
> the displaced_header for the recursive case, before checking whether the monitor is inflated.

I'll figure out an acceptable addition off-thread with you
and then update/add it before pushing. The 72 hour stress
test doesn't finish until Sunday night... so I have time...


> The change looks good.

Thanks!


> I spent a day bringing this back into my memory cache,

I'm hoping that the wikis we're developing will help that process
over time. It's certainly been good to get the lore written down
and out of various folks heads...


> and looking for other similar holes
> and did not find any.

Very cool! I've been planning to audit the places where we grab
ownership of the ObjectMonitor, but I wanted some distance from
the code first...


> I did not do a detailed exploration of AARCH64.

That will be done (by someone) with this bug:

JDK-8153107 enabling ObjectSynchronizer::quick_enter() on ARM64 causes 
hangs
https://bugs.openjdk.java.net/browse/JDK-8153107

Dan


>
> thanks,
> Karen
>
>
>> On Mar 30, 2016, at 7:44 PM, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:
>>
>> Greetings,
>>
>> I've updated the fixes for 8077392 and 8131715 based on code review
>> comments and test results.
>>
>> Here is the webrev URL:
>>
>> http://cr.openjdk.java.net/~dcubed/8077392_8131715-webrev/1-jdk9-hs-rt/
>>
>> src/share/vm/runtime/sharedRuntime.cpp
>>    Changed to disable ObjectSynchronizer::quick_enter() for ARM64
>>    since testing has shown that ARM64 still has hangs when the
>>    quick_enter() optimization is enabled. See:
>>
>>    JDK-8153107 enabling ObjectSynchronizer::quick_enter() on ARM64 causes hangs
>>    https://bugs.openjdk.java.net/browse/JDK-8153107
>>
>>    I suspect a mis-match between the quick_enter() optimization
>>    and the ARM64 MacroAssembler code...
>>
>> src/share/vm/runtime/synchronizer.cpp
>>    Moved the init of the BasicLock's displaced_header to be
>>    unconditional instead of only when Atomic::cmpxchg_ptr()
>>    works. See the more detailed comments.
>>
>>    Also fixed a code review request to rename the 'Lock' param.
>>
>> Redoing all the same testing...
>>
>> As always, comments, suggestions and/or questions are welcome.
>>
>> Dan
>>
>>
>>
>> On 3/22/16 2:41 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> I have fixes for the following two bugs:
>>>
>>>     JDK-8077392 Stream fork/join tasks occasionally fail to complete
>>>     https://bugs.openjdk.java.net/browse/JDK-8077392
>>>
>>>     JDK-8131715 backout the fix for JDK-8079359 when JDK-8077392 is fixed
>>>     https://bugs.openjdk.java.net/browse/JDK-8131715
>>>
>>> Both fixes are very, very small and will be bundled together in the
>>> same changeset for obvious reasons.
>>>
>>> Here is the webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8077392_8131715-webrev/0-jdk9-hs-rt/
>>>
>>> While the fix for JDK-8077392 is a simple 1-liner, the explanation of
>>> the race is much, much longer. I've attached the detailed evaluation
>>> to this RFR; it is a copy of the same note that I added to
>>> https://bugs.openjdk.java.net/browse/JDK-8077392, but the attached
>>> copy has all the indentation white space intact. I don't know why
>>> JBS likes to reformat the notes, but it does... :-(
>>>
>>> Testing:
>>>
>>> - the original failing test is running in a parallel stress config
>>>   on my Solaris X64 server; just under 23 hours and just under
>>>   3000 iterations without a failure in either instance; I'm planning
>>>   to let the stress run go for at least 72 hours.
>>> - RT/SVC nightly equivalent (in progress)
>>>
>>> As always, comments, suggestions and/or questions are welcome.
>>>
>>> Dan