RFR(XXS) 8057750: CTW should not make MH intrinsics not entrant

[Igor Veresov]

Huh, I did exactly the same thing initially, but then decided to just enforce the invariant - it’s hard to weed out the races.

The main one: what if the nmethod is made not entrant for whatever reason after we did the lookup (and possibly compiled if necessary) but before we did the final load from _code to patch the callsite? A good example of where this can happen is a potential safepoint (to redefine classes as in  8056154) after we took the SystemDictionary_lock in find_method_handle_intrinsic(). Perhaps there are other places in the resolution call chain as well. Also, if we enable the sweeper to flush these intrinsics - it can happen at any time at all, fully concurrently.  To solve this, we have to somehow block the ability to make this method not entrant during the resolution process.

The small one (in your solution and in my first one as well): _from_compiled_entry is published after _code, so there is a tiny window when _code != NULL but _from_compile_entry points to the c2i adapter. We may spin and wait for the publication but then there is also a question of how this should interact with the main race. This one is easy to solve though.

Anyways, I agree that having an ability to be able to get rid of these nmethods is very useful and it should be addressed in the future. But I wanted something robust and quickly.

igor

On Sep 8, 2014, at 1:30 AM, Gilles Duboscq <duboscq at ssw.jku.at> wrote:

> Hello,
> 
> In graal we went down a slightly different road to solve this issue:
> instead of trying to find all the possible path through which the VM
> could try to make those intrinsics non-entrant we just re-compile them
> when we need to [1,2].
> 
> While the assert that was added in 8056154 should make it easier to
> spot problems, I think it would be more robust to just add code that
> recompiles the intrinsic such that we don't just crash the VM when we
> could actually do something about it.
> Both approaches are probably complementary.
> 
> -Gilles
> 
> [1] http://hg.openjdk.java.net/graal/graal/rev/b7a1ece4f07b
> [2] http://hg.openjdk.java.net/graal/graal/rev/fefb82b01d6f
> 
> On Mon, Sep 8, 2014 at 8:52 AM, Albert <albert.noll at oracle.com> wrote:
>> Hi Igor,
>> 
>> why are we not allowed to make MH intrinsics not entrant just after we've
>> compiled them?
>> Is this requirement specific to CTW?
>> We do not seem to have such a check in sweeper.cpp, i.e., MH instrinsics are
>> made not-entrant
>> in NMethodSweeper::possibly_flush().
>> 
>> Best,
>> Albert
>> 
>> 
>> On 09/08/2014 08:02 AM, Igor Veresov wrote:
>>> 
>>> Follow-up to 8056154. CTW makes methods that were just compiled not
>>> entrant, which in case of MH intrinsics violates the invariant.
>>> 
>>> Webrev: http://cr.openjdk.java.net/~iveresov/8057750/webrev.00/
>>> 
>>> Thanks!
>>> igor
>> 
>>