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

[Kim Barrett]

On Wed, 25 Nov 2020 18:40:49 GMT, Coleen Phillimore <coleenp 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.

Marked as reviewed by kbarrett (Reviewer).

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.

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

PR: https://git.openjdk.java.net/jdk/pull/1439