[9] RFR (XS): 8139247: Improper locking of MethodData::_extra_data_lock

[David Holmes]

On 4/03/2016 2:49 AM, Daniel D. Daugherty wrote:
> On 3/3/16 3:40 AM, Vladimir Ivanov wrote:
>> http://cr.openjdk.java.net/~vlivanov/8139247/webrev.00
>
> src/share/vm/ci/ciMethodData.cpp
>      Nicely done!

+1 from me.

> We've been bitten by the "MutexLocker /*missing name*/ (lock);" before.
> It's interesting that fixing that runs into JDK-8146616. We should

Yeah - how did Vladimir detect that? Detailed check of the generated code?

> probably make an audit run on finding missing name instances (again).

Yep here's another one in  ./share/vm/utilities/decoder.cpp

DecoderLocker::DecoderLocker() :
   MutexLockerEx(DecoderLocker::is_first_error_thread() ?
                 NULL : Decoder::shared_decoder_lock(), true) {
   _decoder = is_first_error_thread() ?
     Decoder::get_error_handler_instance() : Decoder::get_shared_instance();
   assert(_decoder != NULL, "null decoder");
}

Cheers,
David
-----


> Thumbs up!
>
> Dan
>
>
>> https://bugs.openjdk.java.net/browse/JDK-8139247
>>
>> Solaris Studio C++ compiler generates tightly coupled
>> Monitor::lock/unlock calls for unnamed MutexLocker which breaks
>> synchronization between ciMethodData::load_extra_data() and
>> MethodData::bci_to_extra_data().
>>
>>   MutexLocker(mdo->extra_data_lock());
>>
>> is compiled into:
>>
>>   load_extra_data+0x001d: movq %r13,%rdi
>>   load_extra_data+0x0020: call lock [ 0xfffffd7fe6df46c0, .+0x7cc5c0 ]
>>   load_extra_data+0x0025: movq %r13,%rdi
>>   load_extra_data+0x0028: call unlock [ 0xfffffd7fe6df47f0, .+0x7cc6e8
>>
>> But after adding a name, we hit a compiler bug in SS12u4 and
>> Monitor::unlock call is missed. So, have to add a workaround (same as
>> in JDK-8146616 [1]).
>>
>> Testing: verified machine code for ciMethodData::load_extra_data().
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8146616
>