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

[Vladimir Ivanov]

Albert, Roland,

I take my words back about sweeper.

The method is invalidated during class redefinition because it is 
dependent on redefined class 
(CodeCache::make_marked_nmethods_not_entrant()).

Best regards,
Vladimir Ivanov

On 11/5/13 5:11 PM, Albert Noll wrote:
> Hi Vladimir,
>
> I have a question: the sweeper  calls make_not_entrant() on a nmethod
> only after the nmethod
> has been in the code cache for at least 10 occurrences of a safepoint
> (time_since_reset in sweeper.ccp).
>
> Couldn't it also be that we mistakenly call the function
> make_not_entrant() from somewhere else and that this is the root cause
> of the error?
>
>
> Best,
> Albert
>
> On 11/05/2013 01:47 PM, Roland Westrelin wrote:
>> Hi Vladimir,
>>
>>> 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.
>> Does that mean StressNonEntrant is broken?
>>
>>> 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.
>> Would another fix be to have the sweeper check that the method was
>> made ready to execute before it attempts to make it not entrant?
>>
>> i.e. nmethod->method()->code() == nmethod
>>
>> Roland.
>