On Wed, 25 Nov 2020 23:25:55 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> The ServiceThread cleaning used a stale ObjectFree state when calling
>> remove_dead_entries, because another thread had concurrently set is_enabled
>> to false. Add a lock around setting/resetting the lock event state and
>> retest the state under a lock. Ran the test 100s of time without failure,
>> where otherwise it fails very quickly.
>> Tested with tier2,3 and running tiers 4,5,6 in progress.
>> Thanks to Kim for his previous feedback.
>
> src/hotspot/share/prims/jvmtiEventController.cpp line 463:
>
>> 461: ((now_enabled & OBJECT_FREE_BIT)) != 0) {
>> 462: // Set/reset the event enabled under the tagmap lock.
>> 463: set_enabled_events_with_lock(env, now_enabled);
>
> You could tighten up the test to only handle specially when the state of the
> ObjectFree event is changing, i.e.
>
> (((was_enabled ^ now_enabled) & OBJECT_FREE_BIT) != 0)```
>
> Or you could not bother with the conditionalization at all, and just always
> call set_enabled_events_with_lock; I bet nobody would notice any performance
> difference. That would eliminate "benign" races between unlocked bit setting
> here and bit testing in remove_dead_entries_locked. Of course, the current
> implementation has such races on these bits all over the place; what's one
> race more or less among friends...
>
> Or you could just leave it as you have it. Your call.
I like the idea of reseting the enabled bits under a lock unconditionally.
I'll do that.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1439
