RFR (XS): 8023037 : Race between ciEnv::register_method and nmethod::make_not_entrant_or_zombie

[Vladimir Kozlov]

Note nmethodLocker is not lock:

void nmethodLocker::lock_nmethod(nmethod* nm, bool zombie_ok) {
   if (nm == NULL)  return;
   Atomic::inc(&nm->_lock_count);
   guarantee(zombie_ok || !nm->is_zombie(), "cannot lock a zombie method");
}

The guard is nm->is_locked_by_vm() but neither 
make_not_entrant_or_zombie () or register_method() use it.

So how nmethodLocker helps here?

Vladimir

On 11/5/13 12:42 PM, Vladimir Ivanov wrote:
> Thanks! Missed that one. Fixed.
>
> http://cr.openjdk.java.net/~vlivanov/8023037/webrev.01
>
> Best regards,
> Vladimir Ivanov
>
> On 11/5/13 11:25 PM, Vladimir Kozlov wrote:
>> Should be
>>
>> http://cr.openjdk.java.net/~vlivanov/8023037/webrev.01/
>>
>> What about this place:
>>
>> src/cpu/sparc/vm/sharedRuntime_sparc.cpp:  if (StressNonEntrant) {
>>
>> Vladimir
>>
>> On 11/5/13 11:10 AM, Vladimir Ivanov wrote:
>>> Updated fix:
>>> http://cr.openjdk.java.net/~vlivanov/8023037/webrev.00/
>>>
>>> Removed broken StressNonEntrant code.
>>> Updated comments.
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 11/5/13 3:39 PM, Vladimir Ivanov wrote:
>>>> http://cr.openjdk.java.net/~vlivanov/8023037/webrev.00/
>>>>
>>>> There's a race between compiler thread during method registration and
>>>> sweeper: sweeper can invalidate a nmethod which hasn't been installed
>>>> yet.
>>>>
>>>> Consider the following scenario:
>>>>    ciEnv::register_method:
>>>>      - new nmethod(...)
>>>>
>>>>    sweeper:
>>>>      - invalidates newly allocated nmethod and patches VEP to call
>>>> handle_wrong_method
>>>>      - tries to unlink it, but method()->from_compiled_entry() !=
>>>> verified_entry_point() since nmethod hasn't been installed yet
>>>>
>>>>    ciEnv::register_method:
>>>>      - installs already invalidated nmethod
>>>>
>>>> Calling corresponding Java method will lead to infinite loop: VEP of
>>>> the
>>>> compiled method calls handle_wrong_method and call site resolution
>>>> returns the very same compiled method.
>>>>
>>>> The fix is to grab a lock after nmethod is allocated in the code cache
>>>> and check that it hasn't been already invalidated by the sweeper before
>>>> proceeding. Sweeper already synchronizes on a nmethod before
>>>> invalidating it.
>>>>
>>>> Testing: failing test w/ diagnostic output.
>>>>
>>>> Thanks!
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov