RFR: 8256830: misc tests failed with "assert(env->is_enabled(JVMTI_EVENT_OBJECT_FREE)) failed: checking"

[Coleen Phillimore]

On Wed, 25 Nov 2020 23:25:55 GMT, Kim Barrett <kbarrett at 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