RFR: 8358821: patch_verified_entry causes problems, use nmethod entry barriers instead [v4]

[Erik Österlund]

On Tue, 17 Jun 2025 00:02:29 GMT, Dean Long <dlong at openjdk.org> wrote:

>> src/hotspot/share/gc/z/zBarrierSetNMethod.cpp line 114:
>> 
>>> 112:   // Preserve the sticky bit
>>> 113:   if (is_not_entrant(nm)) {
>>> 114:     value |= not_entrant;
>> 
>> Is it possible to have a race where another thread sets an nmethod to not entrant and the thread calling this making the nmethod entry barrier not entrant?
>> 
>> If this was called to disarm a method and then enter it, it seems a bit sneaky in that case that we pass the nmethod entry barrier even though we under the lock see that it is not entrant. Probably okay but still feels like it might be more robust if the thread setting an nmethod to not entrant is always the one that arms the nmethod entry barrier.
>
> If I understand your concern correctly, there is no race.  The only caller of BarrierSetNMethod::make_not_entrant() is nmethod::make_not_entrant(), and it is done inside a NMethodState_lock critical section.  After a call to nmethod::make_not_entrant(), the nmethod entry barrier is armed and stays that way.
> And by design, a disarm only disarms at the inner nmethod_entry_barrier level, not the outer nmethod_stub_entry_barrier level.

My concern is that while thread 1 calls nmethod::make_not_entrant(), thread 2 racingly performs nmethod entry barrier; it makes the is_not_entrant check before it gets updated, but then it gets updated as the per nmethod lock is taken. The GC code "disarms" the GC barrier but in doing so finds that "oh this should be not entrant", but that's sort of not reflected as thread 2 will then proceed with entering the nmethod it just armed as not entrant in the nmethod entry barrier code.
Does that make sense?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25764#discussion_r2154370247