RFR(S): 8148696: Race loading hsdis may cause SIGSEGV

[Nils Eliasson]

Thank you Vladimir!

Best regards,
Nils Eliasson

On 2016-02-08 14:30, Vladimir Ivanov wrote:
> Looks good.
>
> Best regards,
> Vladimir Ivanov
>
> On 2/8/16 4:24 PM, Nils Eliasson wrote:
>> The link to JDK-8071374 was informative, Thanks!
>>
>> Here is a webrev with just a ttyLocker on the missing path
>> (Disassembler::can_decode)
>>
>> Webrev: http://cr.openjdk.java.net/~neliasso/8148696/webrev.02/
>>
>> Best regards,
>> Nils Eliasson
>>
>> On 2016-02-06 08:50, Nils Eliasson wrote:
>>> Hi Vladimir,
>>>
>>> On 2016-02-05 17:54, Vladimir Ivanov wrote:
>>>>> Summary:
>>>>> We have had two crashes on Sparc where two C2 thread simultaneously
>>>>> tries to load the hsdis library. Three of four code paths are 
>>>>> locked by
>>>>> the ttyLocker, but the fourth is open to races.
>>>> I'd suggest to either add ttyLocker on the fourth path or try to move
>>>> it down the call chain to some shared code. (Keep in mind that
>>>> ttyLocker is reentrant!)
>>>
>>> Yes maybe I should just do that. The time in library check and load is
>>> probably insignificant compared to the time spent disassembling and
>>> printing.
>>>
>>>>
>>>>> Solution:
>>>>> I chose to add another lock (mutex) for this purpose and adapted the
>>>>> code so that library_load code works as intented.
>>>> Another mutex will not work - tty_lock has the lowest rank (event):
>>>>   def(tty_lock                     , Mutex  , event, true,
>>>> Monitor::_safepoint_check_never);      // allow to lock in VM
>>>
>>> In my suggested code the ttylocker is not held while loading the
>>> library. The critical section is slightly smaller than before.
>>>
>>>>
>>>> I experimented with that when fixing 8071374, but decided to guard
>>>> everything with tty_lock.
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>
>>> I'll have a look at the bug and post a new webrev.
>>>
>>> Thanks!
>>> Nils Eliasson
>>>
>>>>
>>>> [1]
>>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2015-December/020417.html 
>>>>
>>>>
>>>>
>>>> "The fix is to serialize access to Disassembler on tty_lock.
>>>> Considering most of the calls to Disassembler::decode are performed
>>>> under tty_lock (which has the lowest rank), it's too burdensome to
>>>> introduce a dedicated lock and a new rank to please deadlock
>>>> detection logic."
>>>>
>>>>
>>>>
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8148696
>>>>> Webrev: http://cr.openjdk.java.net/~neliasso/8148696/webrev.01/
>>>>>
>>>>> Regards,
>>>>> Nils Eliasson
>>>
>>