Use RAII for preventing user-code suspensions

In some places we want to prevent a thread from being suspended by a
kForUserCode suspension. Doing this requires gaining a lock,
checking our current suspension state and possibly retrying. To
simplify this we added a new RAII ScopedNoUserCodeSuspension
capability that does the needed checks.

Test: ./test.py --host
Change-Id: I48c08bc8f99b3574d241e7bfc6945b3358b6d082

4 files changed, 252 insertions, 264 deletions

diff --git a/openjdkjvmti/events.cc b/openjdkjvmti/events.cc
index beb864be23..4f978444f8 100644
--- a/openjdkjvmti/events.cc
+++ b/openjdkjvmti/events.cc
@@ -59,6 +59,7 @@
 #include "thread-inl.h"
 #include "thread_list.h"
 #include "ti_phase.h"
+#include "ti_thread.h"
 #include "well_known_classes.h"
 
 namespace openjdkjvmti {
@@ -1274,59 +1275,48 @@ jvmtiError EventHandler::SetEvent(ArtJvmTiEnv* env,
 
   art::Thread* self = art::Thread::Current();
   art::Thread* target = nullptr;
-  do {
-    ThreadUtil::SuspendCheck(self);
-    art::MutexLock ucsl_mu(self, *art::Locks::user_code_suspension_lock_);
-    // Make sure we won't be suspended in the middle of holding the
-    // thread_suspend_count_lock_ by a user-code suspension. We retry and do
-    // another SuspendCheck to clear this.
-    if (ThreadUtil::WouldSuspendForUserCodeLocked(self)) {
-      continue;
-    }
-    bool old_state;
-    bool new_state;
-    {
-      // From now on we know we cannot get suspended by user-code.
-      // NB This does a SuspendCheck (during thread state change) so we need to
-      // make sure we don't have the 'suspend_lock' locked here.
-      art::ScopedObjectAccess soa(self);
-      art::WriterMutexLock el_mu(self, envs_lock_);
-      art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_);
-      jvmtiError err = ERR(INTERNAL);
-      if (thread != nullptr) {
-        if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) {
-          return err;
-        } else if (target->IsStillStarting() ||
-                  target->GetState() == art::ThreadState::kStarting) {
-          target->Dump(LOG_STREAM(WARNING) << "Is not alive: ");
-          return ERR(THREAD_NOT_ALIVE);
-        }
-      }
+  ScopedNoUserCodeSuspension snucs(self);
 
-
-      {
-        art::WriterMutexLock ei_mu(self, env->event_info_mutex_);
-        old_state = global_mask.Test(event);
-        if (mode == JVMTI_ENABLE) {
-          env->event_masks.EnableEvent(env, target, event);
-          global_mask.Set(event);
-          new_state = true;
-        } else {
-          DCHECK_EQ(mode, JVMTI_DISABLE);
-
-          env->event_masks.DisableEvent(env, target, event);
-          RecalculateGlobalEventMaskLocked(event);
-          new_state = global_mask.Test(event);
-        }
+  bool old_state;
+  bool new_state;
+  {
+    // From now on we know we cannot get suspended by user-code.
+    // NB This does a SuspendCheck (during thread state change) so we need to
+    // make sure we don't have the 'suspend_lock' locked here.
+    art::ScopedObjectAccess soa(self);
+    art::WriterMutexLock el_mu(self, envs_lock_);
+    art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_);
+    jvmtiError err = ERR(INTERNAL);
+    if (thread != nullptr) {
+      if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) {
+        return err;
+      } else if (target->IsStillStarting() ||
+                target->GetState() == art::ThreadState::kStarting) {
+        target->Dump(LOG_STREAM(WARNING) << "Is not alive: ");
+        return ERR(THREAD_NOT_ALIVE);
       }
     }
-    // Handle any special work required for the event type. We still have the
-    // user_code_suspend_count_lock_ so there won't be any interleaving here.
-    if (new_state != old_state) {
-      return HandleEventType(event, thread, mode == JVMTI_ENABLE);
+
+    art::WriterMutexLock ei_mu(self, env->event_info_mutex_);
+    old_state = global_mask.Test(event);
+    if (mode == JVMTI_ENABLE) {
+      env->event_masks.EnableEvent(env, target, event);
+      global_mask.Set(event);
+      new_state = true;
+    } else {
+      DCHECK_EQ(mode, JVMTI_DISABLE);
+
+      env->event_masks.DisableEvent(env, target, event);
+      RecalculateGlobalEventMaskLocked(event);
+      new_state = global_mask.Test(event);
     }
-    return OK;
-  } while (true);
+  }
+  // Handle any special work required for the event type. We still have the
+  // user_code_suspend_count_lock_ so there won't be any interleaving here.
+  if (new_state != old_state) {
+    return HandleEventType(event, thread, mode == JVMTI_ENABLE);
+  }
+  return OK;
 }
 
 void EventHandler::HandleBreakpointEventsChanged(bool added) {
diff --git a/openjdkjvmti/ti_stack.cc b/openjdkjvmti/ti_stack.cc
index b96c92aac7..6ad4e2a84d 100644
--- a/openjdkjvmti/ti_stack.cc
+++ b/openjdkjvmti/ti_stack.cc
@@ -1014,153 +1014,144 @@ jvmtiError StackUtil::NotifyFramePop(jvmtiEnv* env, jthread thread, jint depth)
   ArtJvmTiEnv* tienv = ArtJvmTiEnv::AsArtJvmTiEnv(env);
   art::Thread* self = art::Thread::Current();
   art::Thread* target;
-  do {
-    ThreadUtil::SuspendCheck(self);
-    art::MutexLock ucsl_mu(self, *art::Locks::user_code_suspension_lock_);
-    // Make sure we won't be suspended in the middle of holding the thread_suspend_count_lock_ by a
-    // user-code suspension. We retry and do another SuspendCheck to clear this.
-    if (ThreadUtil::WouldSuspendForUserCodeLocked(self)) {
-      continue;
-    }
-    // From now on we know we cannot get suspended by user-code.
-    // NB This does a SuspendCheck (during thread state change) so we need to make sure we don't
-    // have the 'suspend_lock' locked here.
-    art::ScopedObjectAccess soa(self);
-    art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_);
-    jvmtiError err = ERR(INTERNAL);
-    if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) {
-      return err;
-    }
-    if (target != self) {
-      // TODO This is part of the spec but we could easily avoid needing to do it. We would just put
-      // all the logic into a sync-checkpoint.
-      art::MutexLock tscl_mu(self, *art::Locks::thread_suspend_count_lock_);
-      if (target->GetUserCodeSuspendCount() == 0) {
-        return ERR(THREAD_NOT_SUSPENDED);
-      }
-    }
-    // We hold the user_code_suspension_lock_ so the target thread is staying suspended until we are
-    // done (unless it's 'self' in which case we don't care since we aren't going to be returning).
-    // TODO We could implement this using a synchronous checkpoint and not bother with any of the
-    // suspension stuff. The spec does specifically say to return THREAD_NOT_SUSPENDED though.
-    // Find the requested stack frame.
-    std::unique_ptr<art::Context> context(art::Context::Create());
-    FindFrameAtDepthVisitor visitor(target, context.get(), depth);
-    visitor.WalkStack();
-    if (!visitor.FoundFrame()) {
-      return ERR(NO_MORE_FRAMES);
-    }
-    art::ArtMethod* method = visitor.GetMethod();
-    if (method->IsNative()) {
-      return ERR(OPAQUE_FRAME);
-    }
-    // From here we are sure to succeed.
-    bool needs_instrument = false;
-    // Get/create a shadow frame
-    art::ShadowFrame* shadow_frame = visitor.GetOrCreateShadowFrame(&needs_instrument);
-    {
-      art::WriterMutexLock lk(self, tienv->event_info_mutex_);
-      if (LIKELY(!shadow_frame->NeedsNotifyPop())) {
-        // Ensure we won't miss exceptions being thrown if we get jit-compiled. We only do this for
-        // the first NotifyPopFrame.
-        target->IncrementForceInterpreterCount();
-
-        // Mark shadow frame as needs_notify_pop_
-        shadow_frame->SetNotifyPop(true);
-      }
-      tienv->notify_frames.insert(shadow_frame);
+
+  ScopedNoUserCodeSuspension snucs(self);
+  // From now on we know we cannot get suspended by user-code.
+  // NB This does a SuspendCheck (during thread state change) so we need to make
+  // sure we don't have the 'suspend_lock' locked here.
+  art::ScopedObjectAccess soa(self);
+  art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_);
+  jvmtiError err = ERR(INTERNAL);
+  if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) {
+    return err;
+  }
+  if (target != self) {
+    // TODO This is part of the spec but we could easily avoid needing to do it.
+    // We would just put all the logic into a sync-checkpoint.
+    art::MutexLock tscl_mu(self, *art::Locks::thread_suspend_count_lock_);
+    if (target->GetUserCodeSuspendCount() == 0) {
+      return ERR(THREAD_NOT_SUSPENDED);
     }
-    // Make sure can we will go to the interpreter and use the shadow frames.
-    if (needs_instrument) {
-      art::Runtime::Current()->GetInstrumentation()->InstrumentThreadStack(target);
+  }
+  // We hold the user_code_suspension_lock_ so the target thread is staying
+  // suspended until we are done (unless it's 'self' in which case we don't care
+  // since we aren't going to be returning).
+  // TODO We could implement this using a synchronous checkpoint and not bother
+  // with any of the suspension stuff. The spec does specifically say to return
+  // THREAD_NOT_SUSPENDED though. Find the requested stack frame.
+  std::unique_ptr<art::Context> context(art::Context::Create());
+  FindFrameAtDepthVisitor visitor(target, context.get(), depth);
+  visitor.WalkStack();
+  if (!visitor.FoundFrame()) {
+    return ERR(NO_MORE_FRAMES);
+  }
+  art::ArtMethod* method = visitor.GetMethod();
+  if (method->IsNative()) {
+    return ERR(OPAQUE_FRAME);
+  }
+  // From here we are sure to succeed.
+  bool needs_instrument = false;
+  // Get/create a shadow frame
+  art::ShadowFrame* shadow_frame =
+      visitor.GetOrCreateShadowFrame(&needs_instrument);
+  {
+    art::WriterMutexLock lk(self, tienv->event_info_mutex_);
+    if (LIKELY(!shadow_frame->NeedsNotifyPop())) {
+      // Ensure we won't miss exceptions being thrown if we get jit-compiled. We
+      // only do this for the first NotifyPopFrame.
+      target->IncrementForceInterpreterCount();
+
+      // Mark shadow frame as needs_notify_pop_
+      shadow_frame->SetNotifyPop(true);
     }
-    return OK;
-  } while (true);
+    tienv->notify_frames.insert(shadow_frame);
+  }
+  // Make sure can we will go to the interpreter and use the shadow frames.
+  if (needs_instrument) {
+    art::Runtime::Current()->GetInstrumentation()->InstrumentThreadStack(
+        target);
+  }
+  return OK;
 }
 
 jvmtiError StackUtil::PopFrame(jvmtiEnv* env, jthread thread) {
   art::Thread* self = art::Thread::Current();
   art::Thread* target;
-  do {
-    ThreadUtil::SuspendCheck(self);
-    art::MutexLock ucsl_mu(self, *art::Locks::user_code_suspension_lock_);
-    // Make sure we won't be suspended in the middle of holding the thread_suspend_count_lock_ by a
-    // user-code suspension. We retry and do another SuspendCheck to clear this.
-    if (ThreadUtil::WouldSuspendForUserCodeLocked(self)) {
-      continue;
-    }
-    // From now on we know we cannot get suspended by user-code.
-    // NB This does a SuspendCheck (during thread state change) so we need to make sure we don't
-    // have the 'suspend_lock' locked here.
-    art::ScopedObjectAccess soa(self);
-    art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_);
-    jvmtiError err = ERR(INTERNAL);
-    if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) {
-      return err;
-    }
-    {
-      art::MutexLock tscl_mu(self, *art::Locks::thread_suspend_count_lock_);
-      if (target == self || target->GetUserCodeSuspendCount() == 0) {
-        // We cannot be the current thread for this function.
-        return ERR(THREAD_NOT_SUSPENDED);
-      }
-    }
-    JvmtiGlobalTLSData* tls_data = ThreadUtil::GetGlobalTLSData(target);
-    constexpr art::StackVisitor::StackWalkKind kWalkKind =
-        art::StackVisitor::StackWalkKind::kIncludeInlinedFrames;
-    if (tls_data != nullptr &&
-        tls_data->disable_pop_frame_depth != JvmtiGlobalTLSData::kNoDisallowedPopFrame &&
-        tls_data->disable_pop_frame_depth == art::StackVisitor::ComputeNumFrames(target,
-                                                                                 kWalkKind)) {
-      JVMTI_LOG(WARNING, env) << "Disallowing frame pop due to in-progress class-load/prepare. "
-                              << "Frame at depth " << tls_data->disable_pop_frame_depth << " was "
-                              << "marked as un-poppable by the jvmti plugin. See b/117615146 for "
-                              << "more information.";
-      return ERR(OPAQUE_FRAME);
-    }
-    // We hold the user_code_suspension_lock_ so the target thread is staying suspended until we are
-    // done.
-    std::unique_ptr<art::Context> context(art::Context::Create());
-    FindFrameAtDepthVisitor final_frame(target, context.get(), 0);
-    FindFrameAtDepthVisitor penultimate_frame(target, context.get(), 1);
-    final_frame.WalkStack();
-    penultimate_frame.WalkStack();
-
-    if (!final_frame.FoundFrame() || !penultimate_frame.FoundFrame()) {
-      // Cannot do it if there is only one frame!
-      return ERR(NO_MORE_FRAMES);
-    }
 
-    art::ArtMethod* called_method = final_frame.GetMethod();
-    art::ArtMethod* calling_method = penultimate_frame.GetMethod();
-    if (calling_method->IsNative() || called_method->IsNative()) {
-      return ERR(OPAQUE_FRAME);
-    }
-    // From here we are sure to succeed.
-
-    // Get/create a shadow frame
-    bool created_final_frame = false;
-    bool created_penultimate_frame = false;
-    art::ShadowFrame* called_shadow_frame =
-        final_frame.GetOrCreateShadowFrame(&created_final_frame);
-    art::ShadowFrame* calling_shadow_frame =
-        penultimate_frame.GetOrCreateShadowFrame(&created_penultimate_frame);
-
-    CHECK_NE(called_shadow_frame, calling_shadow_frame)
-        << "Frames at different depths not different!";
-
-    // Tell the shadow-frame to return immediately and skip all exit events.
-    called_shadow_frame->SetForcePopFrame(true);
-    calling_shadow_frame->SetForceRetryInstruction(true);
-
-    // Make sure can we will go to the interpreter and use the shadow frames. The early return for
-    // the final frame will force everything to the interpreter so we only need to instrument if it
-    // was not present.
-    if (created_final_frame) {
-      DeoptManager::Get()->DeoptimizeThread(target);
+  ScopedNoUserCodeSuspension snucs(self);
+  // From now on we know we cannot get suspended by user-code.
+  // NB This does a SuspendCheck (during thread state change) so we need to make
+  // sure we don't have the 'suspend_lock' locked here.
+  art::ScopedObjectAccess soa(self);
+  art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_);
+  jvmtiError err = ERR(INTERNAL);
+  if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) {
+    return err;
+  }
+  {
+    art::MutexLock tscl_mu(self, *art::Locks::thread_suspend_count_lock_);
+    if (target == self || target->GetUserCodeSuspendCount() == 0) {
+      // We cannot be the current thread for this function.
+      return ERR(THREAD_NOT_SUSPENDED);
     }
-    return OK;
-  } while (true);
+  }
+  JvmtiGlobalTLSData* tls_data = ThreadUtil::GetGlobalTLSData(target);
+  constexpr art::StackVisitor::StackWalkKind kWalkKind =
+      art::StackVisitor::StackWalkKind::kIncludeInlinedFrames;
+  if (tls_data != nullptr &&
+      tls_data->disable_pop_frame_depth !=
+          JvmtiGlobalTLSData::kNoDisallowedPopFrame &&
+      tls_data->disable_pop_frame_depth ==
+          art::StackVisitor::ComputeNumFrames(target, kWalkKind)) {
+    JVMTI_LOG(WARNING, env)
+        << "Disallowing frame pop due to in-progress class-load/prepare. "
+        << "Frame at depth " << tls_data->disable_pop_frame_depth << " was "
+        << "marked as un-poppable by the jvmti plugin. See b/117615146 for "
+        << "more information.";
+    return ERR(OPAQUE_FRAME);
+  }
+  // We hold the user_code_suspension_lock_ so the target thread is staying
+  // suspended until we are done.
+  std::unique_ptr<art::Context> context(art::Context::Create());
+  FindFrameAtDepthVisitor final_frame(target, context.get(), 0);
+  FindFrameAtDepthVisitor penultimate_frame(target, context.get(), 1);
+  final_frame.WalkStack();
+  penultimate_frame.WalkStack();
+
+  if (!final_frame.FoundFrame() || !penultimate_frame.FoundFrame()) {
+    // Cannot do it if there is only one frame!
+    return ERR(NO_MORE_FRAMES);
+  }
+
+  art::ArtMethod* called_method = final_frame.GetMethod();
+  art::ArtMethod* calling_method = penultimate_frame.GetMethod();
+  if (calling_method->IsNative() || called_method->IsNative()) {
+    return ERR(OPAQUE_FRAME);
+  }
+  // From here we are sure to succeed.
+
+  // Get/create a shadow frame
+  bool created_final_frame = false;
+  bool created_penultimate_frame = false;
+  art::ShadowFrame* called_shadow_frame =
+      final_frame.GetOrCreateShadowFrame(&created_final_frame);
+  art::ShadowFrame* calling_shadow_frame =
+      penultimate_frame.GetOrCreateShadowFrame(&created_penultimate_frame);
+
+  CHECK_NE(called_shadow_frame, calling_shadow_frame)
+      << "Frames at different depths not different!";
+
+  // Tell the shadow-frame to return immediately and skip all exit events.
+  called_shadow_frame->SetForcePopFrame(true);
+  calling_shadow_frame->SetForceRetryInstruction(true);
+
+  // Make sure can we will go to the interpreter and use the shadow frames. The
+  // early return for the final frame will force everything to the interpreter
+  // so we only need to instrument if it was not present.
+  if (created_final_frame) {
+    DeoptManager::Get()->DeoptimizeThread(target);
+  }
+  return OK;
 }
 
 }  // namespace openjdkjvmti
diff --git a/openjdkjvmti/ti_thread.cc b/openjdkjvmti/ti_thread.cc
index 01cc8c7699..15fdfb3ab5 100644
--- a/openjdkjvmti/ti_thread.cc
+++ b/openjdkjvmti/ti_thread.cc
@@ -65,6 +65,30 @@ static const char* kJvmtiTlsKey = "JvmtiTlsKey";
 
 art::ArtField* ThreadUtil::context_class_loader_ = nullptr;
 
+ScopedNoUserCodeSuspension::ScopedNoUserCodeSuspension(art::Thread* self) : self_(self) {
+  DCHECK_EQ(self, art::Thread::Current());
+  // Loop until we both have the user_code_suspension_locK_ and don't have any pending user_code
+  // suspensions.
+  do {
+    art::Locks::user_code_suspension_lock_->AssertNotHeld(self_);
+    ThreadUtil::SuspendCheck(self_);
+
+    art::Locks::user_code_suspension_lock_->ExclusiveLock(self_);
+    if (ThreadUtil::WouldSuspendForUserCodeLocked(self_)) {
+      art::Locks::user_code_suspension_lock_->ExclusiveUnlock(self_);
+      continue;
+    }
+
+    art::Locks::user_code_suspension_lock_->AssertHeld(self_);
+
+    return;
+  } while (true);
+}
+
+ScopedNoUserCodeSuspension::~ScopedNoUserCodeSuspension() {
+  art::Locks::user_code_suspension_lock_->ExclusiveUnlock(self_);
+}
+
 struct ThreadCallback : public art::ThreadLifecycleCallback {
   jthread GetThreadObject(art::Thread* self) REQUIRES_SHARED(art::Locks::mutator_lock_) {
     if (self->GetPeer() == nullptr) {
@@ -544,17 +568,8 @@ jvmtiError ThreadUtil::GetThreadState(jvmtiEnv* env ATTRIBUTE_UNUSED,
 
   art::Thread* self = art::Thread::Current();
   InternalThreadState state = {};
-  // Loop since we need to bail out and try again if we would end up getting suspended while holding
-  // the user_code_suspension_lock_ due to a SuspendReason::kForUserCode. In this situation we
-  // release the lock, wait to get resumed and try again.
-  do {
-    SuspendCheck(self);
-    art::MutexLock ucsl_mu(self, *art::Locks::user_code_suspension_lock_);
-    if (WouldSuspendForUserCodeLocked(self)) {
-      // Make sure we won't be suspended in the middle of holding the thread_suspend_count_lock_ by
-      // a user-code suspension. We retry and do another SuspendCheck to clear this.
-      continue;
-    }
+  {
+    ScopedNoUserCodeSuspension snucs(self);
     art::ScopedObjectAccess soa(self);
     art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_);
     jvmtiError err = ERR(INTERNAL);
@@ -563,24 +578,23 @@ jvmtiError ThreadUtil::GetThreadState(jvmtiEnv* env ATTRIBUTE_UNUSED,
       return err;
     }
     state = GetNativeThreadState(target);
-    if (state.art_state == art::ThreadState::kStarting) {
-      break;
-    }
-    DCHECK(state.native_thread != nullptr);
+    if (state.art_state != art::ThreadState::kStarting) {
+      DCHECK(state.native_thread != nullptr);
 
-    // Translate internal thread state to JVMTI and Java state.
-    jint jvmti_state = GetJvmtiThreadStateFromInternal(state);
+      // Translate internal thread state to JVMTI and Java state.
+      jint jvmti_state = GetJvmtiThreadStateFromInternal(state);
 
-    // Java state is derived from nativeGetState.
-    // TODO: Our implementation assigns "runnable" to suspended. As such, we will have slightly
-    //       different mask if a thread got suspended due to user-code. However, this is for
-    //       consistency with the Java view.
-    jint java_state = GetJavaStateFromInternal(state);
+      // Java state is derived from nativeGetState.
+      // TODO: Our implementation assigns "runnable" to suspended. As such, we will have slightly
+      //       different mask if a thread got suspended due to user-code. However, this is for
+      //       consistency with the Java view.
+      jint java_state = GetJavaStateFromInternal(state);
 
-    *thread_state_ptr = jvmti_state | java_state;
+      *thread_state_ptr = jvmti_state | java_state;
 
-    return ERR(NONE);
-  } while (true);
+      return ERR(NONE);
+    }
+  }
 
   DCHECK_EQ(state.art_state, art::ThreadState::kStarting);
 
@@ -857,18 +871,7 @@ jvmtiError ThreadUtil::SuspendOther(art::Thread* self,
   // the user_code_suspension_lock_ due to a SuspendReason::kForUserCode. In this situation we
   // release the lock, wait to get resumed and try again.
   do {
-    // Suspend ourself if we have any outstanding suspends. This is so we won't suspend due to
-    // another SuspendThread in the middle of suspending something else potentially causing a
-    // deadlock. We need to do this in the loop because if we ended up back here then we had
-    // outstanding SuspendReason::kForUserCode suspensions and we should wait for them to be cleared
-    // before continuing.
-    SuspendCheck(self);
-    art::MutexLock mu(self, *art::Locks::user_code_suspension_lock_);
-    if (WouldSuspendForUserCodeLocked(self)) {
-      // Make sure we won't be suspended in the middle of holding the thread_suspend_count_lock_ by
-      // a user-code suspension. We retry and do another SuspendCheck to clear this.
-      continue;
-    }
+    ScopedNoUserCodeSuspension snucs(self);
     // We are not going to be suspended by user code from now on.
     {
       art::ScopedObjectAccess soa(self);
@@ -957,51 +960,44 @@ jvmtiError ThreadUtil::ResumeThread(jvmtiEnv* env ATTRIBUTE_UNUSED,
   }
   art::Thread* self = art::Thread::Current();
   art::Thread* target;
-  // Retry until we know we won't get suspended by user code while resuming something.
-  do {
-    SuspendCheck(self);
-    art::MutexLock ucsl_mu(self, *art::Locks::user_code_suspension_lock_);
-    if (WouldSuspendForUserCodeLocked(self)) {
-      // Make sure we won't be suspended in the middle of holding the thread_suspend_count_lock_ by
-      // a user-code suspension. We retry and do another SuspendCheck to clear this.
-      continue;
+
+  // Make sure we won't get suspended ourselves while in the middle of resuming another thread.
+  ScopedNoUserCodeSuspension snucs(self);
+  // From now on we know we cannot get suspended by user-code.
+  {
+    // NB This does a SuspendCheck (during thread state change) so we need to make sure we don't
+    // have the 'suspend_lock' locked here.
+    art::ScopedObjectAccess soa(self);
+    art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_);
+    jvmtiError err = ERR(INTERNAL);
+    if (!GetAliveNativeThread(thread, soa, &target, &err)) {
+      return err;
+    } else if (target == self) {
+      // We would have paused until we aren't suspended anymore due to the ScopedObjectAccess so
+      // we can just return THREAD_NOT_SUSPENDED. Unfortunately we cannot do any real DCHECKs
+      // about current state since it's all concurrent.
+      return ERR(THREAD_NOT_SUSPENDED);
     }
-    // From now on we know we cannot get suspended by user-code.
+    // The JVMTI spec requires us to return THREAD_NOT_SUSPENDED if it is alive but we really
+    // cannot tell why resume failed.
     {
-      // NB This does a SuspendCheck (during thread state change) so we need to make sure we don't
-      // have the 'suspend_lock' locked here.
-      art::ScopedObjectAccess soa(self);
-      art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_);
-      jvmtiError err = ERR(INTERNAL);
-      if (!GetAliveNativeThread(thread, soa, &target, &err)) {
-        return err;
-      } else if (target == self) {
-        // We would have paused until we aren't suspended anymore due to the ScopedObjectAccess so
-        // we can just return THREAD_NOT_SUSPENDED. Unfortunately we cannot do any real DCHECKs
-        // about current state since it's all concurrent.
+      art::MutexLock thread_suspend_count_mu(self, *art::Locks::thread_suspend_count_lock_);
+      if (target->GetUserCodeSuspendCount() == 0) {
         return ERR(THREAD_NOT_SUSPENDED);
       }
-      // The JVMTI spec requires us to return THREAD_NOT_SUSPENDED if it is alive but we really
-      // cannot tell why resume failed.
-      {
-        art::MutexLock thread_suspend_count_mu(self, *art::Locks::thread_suspend_count_lock_);
-        if (target->GetUserCodeSuspendCount() == 0) {
-          return ERR(THREAD_NOT_SUSPENDED);
-        }
-      }
-    }
-    // It is okay that we don't have a thread_list_lock here since we know that the thread cannot
-    // die since it is currently held suspended by a SuspendReason::kForUserCode suspend.
-    DCHECK(target != self);
-    if (!art::Runtime::Current()->GetThreadList()->Resume(target,
-                                                          art::SuspendReason::kForUserCode)) {
-      // TODO Give a better error.
-      // This is most likely THREAD_NOT_SUSPENDED but we cannot really be sure.
-      return ERR(INTERNAL);
-    } else {
-      return OK;
     }
-  } while (true);
+  }
+  // It is okay that we don't have a thread_list_lock here since we know that the thread cannot
+  // die since it is currently held suspended by a SuspendReason::kForUserCode suspend.
+  DCHECK(target != self);
+  if (!art::Runtime::Current()->GetThreadList()->Resume(target,
+                                                        art::SuspendReason::kForUserCode)) {
+    // TODO Give a better error.
+    // This is most likely THREAD_NOT_SUSPENDED but we cannot really be sure.
+    return ERR(INTERNAL);
+  } else {
+    return OK;
+  }
 }
 
 static bool IsCurrentThread(jthread thr) {
diff --git a/openjdkjvmti/ti_thread.h b/openjdkjvmti/ti_thread.h
index 39f1f0725c..c5443bfb9f 100644
--- a/openjdkjvmti/ti_thread.h
+++ b/openjdkjvmti/ti_thread.h
@@ -52,6 +52,17 @@ namespace openjdkjvmti {
 
 class EventHandler;
 
+// Gains the user_code_suspension_lock_ and ensures that the code will not suspend for user-code.
+class SCOPED_CAPABILITY ScopedNoUserCodeSuspension {
+ public:
+  explicit ScopedNoUserCodeSuspension(art::Thread* self)
+      ACQUIRE(art::Locks::user_code_suspension_lock_);
+  ~ScopedNoUserCodeSuspension() RELEASE(art::Locks::user_code_suspension_lock_);
+
+ private:
+  art::Thread* self_;
+};
+
 // The struct that we store in the art::Thread::custom_tls_ that maps the jvmtiEnvs to the data
 // stored with that thread. This is needed since different jvmtiEnvs are not supposed to share TLS
 // data but we only have a single slot in Thread objects to store data.