[9] RFR(S): 8153134: Infinite loop in handle_wrong_method in jmod

[dean.long at oracle.com]

+1

dl


On 10/12/16 10:55 AM, Vladimir Kozlov wrote:
> Looks good.
>
> Thanks,
> Vladimir
>
> On 10/12/16 12:04 AM, Tobias Hartmann wrote:
>> Hi,
>>
>> please review the following patch:
>> https://bugs.openjdk.java.net/browse/JDK-8153134
>> http://cr.openjdk.java.net/~thartmann/8153134/webrev.01/
>>
>> There is a race between Method::set_code() and Method::clear_code() 
>> that may cause a Method to end up in an inconsistent state. As a 
>> result, we loop forever in handle_wrong_method() trying to resolve a 
>> call to this Method.
>>
>> In more detail, we have method A that calls method B and the 
>> following threads:
>> - Thread T1: CompilerThread that is compiling method B and currently 
>> registers the nmethodB.
>> - Thread T2: Currently executes nmethodA.
>>
>> Here is the execution sequence:
>> T1: Registers nmethodB in Method::set_code():
>>     _code = nmethodB
>> T2: Resolves call to method B in CompiledIC::compute_monomorphic_entry:
>>     Since Method::_code is set, entry is set to 
>> nmethodB::_verified_entry_point
>> T2: nmethodB is converted to zombie in 
>> nmethodB::make_not_entrant_or_zombie():
>>     The verified entry is patched to jump to 
>> handle_wrong_method_stub() and Method::clear_code() is called:
>>     _from_compiled_entry = c2i
>>     _from_interpreted_entry = i2i
>> T1: Continues in Method::set_code():
>>     _from_compiled_entry = _verified_entry_point
>>     _from_interpreted_entry = i2c
>> T2: Continues in Method::clear_code():
>>     _code = NULL
>>
>> Method B ends up with:
>> _from_compiled_entry = _verified_entry_point
>> _from_interpreted_entry = i2c
>> _code = NULL
>>
>> As a result, handle_wrong_method() returns _from_compiled_entry == 
>> _verified_entry_point which was patched to jump to 
>> handle_wrong_method() again. We loop forever.
>>
>> Unfortunately, I was not able to reproduce the problem but got all 
>> the information from the core file/logs. As Dean suggested, I used 
>> the Patching_lock to synchronize access to these fields. I removed 
>> the call to clear_code() in classLoader.cpp because 
>> make_not_entrant() includes clear_code().
>>
>> I verified that the additional locking does not affect performance. I 
>> also did correctness testing with RBT.
>>
>> Thanks,
>> Tobias
>>