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

[Nils Eliasson]

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
>