Ensure that updates to the global event mask are atomic
We were setting and testing the global event mask in a way that
allowed races with other threads. This could cause issues if multiple
threads write to the mask at the same time and could cause changes to
the current event state to be missed.
Test: ./test.py --host -j50
Bug: 69657830
Change-Id: I5e2759af598e6179fd25fcbe6b211a9369217156
diff --git a/openjdkjvmti/events.cc b/openjdkjvmti/events.cc
index be4ebbc..a3d9bb4 100644
--- a/openjdkjvmti/events.cc
+++ b/openjdkjvmti/events.cc
@@ -139,7 +139,9 @@
}
-void EventMasks::EnableEvent(art::Thread* thread, ArtJvmtiEvent event) {
+void EventMasks::EnableEvent(ArtJvmTiEnv* env, art::Thread* thread, ArtJvmtiEvent event) {
+ DCHECK_EQ(&env->event_masks, this);
+ env->event_info_mutex_.AssertExclusiveHeld(art::Thread::Current());
DCHECK(EventMask::EventIsInRange(event));
GetEventMask(thread).Set(event);
if (thread != nullptr) {
@@ -147,7 +149,9 @@
}
}
-void EventMasks::DisableEvent(art::Thread* thread, ArtJvmtiEvent event) {
+void EventMasks::DisableEvent(ArtJvmTiEnv* env, art::Thread* thread, ArtJvmtiEvent event) {
+ DCHECK_EQ(&env->event_masks, this);
+ env->event_info_mutex_.AssertExclusiveHeld(art::Thread::Current());
DCHECK(EventMask::EventIsInRange(event));
GetEventMask(thread).Set(event, false);
if (thread != nullptr) {
@@ -1131,20 +1135,28 @@
return ERR(MUST_POSSESS_CAPABILITY);
}
- bool old_state = global_mask.Test(event);
+ bool old_state;
+ bool new_state;
- if (mode == JVMTI_ENABLE) {
- env->event_masks.EnableEvent(thread, event);
- global_mask.Set(event);
- } else {
- DCHECK_EQ(mode, JVMTI_DISABLE);
+ {
+ // Change the event masks atomically.
+ art::Thread* self = art::Thread::Current();
+ art::MutexLock mu(self, envs_lock_);
+ art::WriterMutexLock mu_env_info(self, env->event_info_mutex_);
+ old_state = global_mask.Test(event);
+ if (mode == JVMTI_ENABLE) {
+ env->event_masks.EnableEvent(env, thread, event);
+ global_mask.Set(event);
+ new_state = true;
+ } else {
+ DCHECK_EQ(mode, JVMTI_DISABLE);
- env->event_masks.DisableEvent(thread, event);
- RecalculateGlobalEventMask(event);
+ env->event_masks.DisableEvent(env, thread, event);
+ RecalculateGlobalEventMaskLocked(event);
+ new_state = global_mask.Test(event);
+ }
}
- bool new_state = global_mask.Test(event);
-
// Handle any special work required for the event type.
if (new_state != old_state) {
HandleEventType(event, mode == JVMTI_ENABLE);
diff --git a/openjdkjvmti/events.h b/openjdkjvmti/events.h
index c73215f..d21a587 100644
--- a/openjdkjvmti/events.h
+++ b/openjdkjvmti/events.h
@@ -149,8 +149,16 @@
EventMask& GetEventMask(art::Thread* thread);
EventMask* GetEventMaskOrNull(art::Thread* thread);
- void EnableEvent(art::Thread* thread, ArtJvmtiEvent event);
- void DisableEvent(art::Thread* thread, ArtJvmtiEvent event);
+ // Circular dependencies mean we cannot see the definition of ArtJvmTiEnv so the mutex is simply
+ // asserted in the function.
+ // Note that the 'env' passed in must be the same env this EventMasks is associated with.
+ void EnableEvent(ArtJvmTiEnv* env, art::Thread* thread, ArtJvmtiEvent event);
+ // REQUIRES(env->event_info_mutex_);
+ // Circular dependencies mean we cannot see the definition of ArtJvmTiEnv so the mutex is simply
+ // asserted in the function.
+ // Note that the 'env' passed in must be the same env this EventMasks is associated with.
+ void DisableEvent(ArtJvmTiEnv* env, art::Thread* thread, ArtJvmtiEvent event);
+ // REQUIRES(env->event_info_mutex_);
bool IsEnabledAnywhere(ArtJvmtiEvent event);
// Make any changes to event masks needed for the given capability changes. If caps_added is true
// then caps is all the newly set capabilities of the jvmtiEnv. If it is false then caps is the