Address review comments with mixin class

Author: jeffreytan81

lldb/include/lldb/Target/Process.h

@@ -1316,11 +1316,11 @@ class Process : public std::enable_shared_from_this<Process>,
 
   /// Send an async interrupt request.
   ///
-  /// If \a thread is specified the async interrupt stop will be attributed the
-  /// specified thread.
+  /// If \a thread is specified the async interrupt stop will be attributed to
+  /// the specified thread.
   ///
   /// \param[in] thread
-  ///     The thread from which to attribute the async interrupt stop to.
+  ///     The thread the async interrupt will be attributed to.
   void SendAsyncInterrupt(Thread *thread = nullptr);
 
   // Notify this process class that modules got loaded.
@@ -2844,6 +2844,14 @@ void PruneThreadPlans();
     return std::nullopt;
   }
 
+  /// Handle thread specific async interrupt and return the original thread
+  /// that requested the async interrupt. It can be null if original thread
+  /// has exited.
+  virtual lldb::ThreadSP
+  HandleThreadAsyncInterrupt(uint8_t signo, const std::string &description) {
+    return lldb::ThreadSP();
+  }
+
   lldb::StateType GetPrivateState();
 
   /// The "private" side of resuming a process.  This doesn't alter the state

lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h

@@ -46,10 +46,16 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
 
   ~ThreadPlanSingleThreadTimeout() override;
 
-  // Create a new instance from fresh new state.
-  static void CreateNew(Thread &thread, TimeoutInfo &info);
-  // Reset and create a new instance from the previous state.
-  static void ResetFromPrevState(Thread &thread, TimeoutInfo &info);
+  // If input \param thread is running in single thread mode, push a
+  // new ThreadPlanSingleThreadTimeout based on timeout setting from fresh new
+  // state. The reference of \param info is passed in so that when
+  // ThreadPlanSingleThreadTimeout got popped out its last state can be stored
+  // in it for future resume.
+  static void PushNewWithTimeout(Thread &thread, TimeoutInfo &info);
+
+  // Push a new ThreadPlanSingleThreadTimeout by restoring state from
+  // input \param info and resume execution.
+  static void ResumeFromPrevState(Thread &thread, TimeoutInfo &info);
 
   void GetDescription(Stream *s, lldb::DescriptionLevel level) override;
   bool ValidatePlan(Stream *error) override { return true; }

lldb/include/lldb/Target/ThreadPlanStepOverRange.h

@@ -12,13 +12,14 @@
 #include "lldb/Core/AddressRange.h"
 #include "lldb/Target/StackID.h"
 #include "lldb/Target/Thread.h"
-#include "lldb/Target/ThreadPlanSingleThreadTimeout.h"
 #include "lldb/Target/ThreadPlanStepRange.h"
+#include "lldb/Target/TimeoutResumeAll.h"
 
 namespace lldb_private {
 
 class ThreadPlanStepOverRange : public ThreadPlanStepRange,
-                                ThreadPlanShouldStopHere {
+                                ThreadPlanShouldStopHere,
+                                TimeoutResumeAll {
 public:
   ThreadPlanStepOverRange(Thread &thread, const AddressRange &range,
                           const SymbolContext &addr_context,
@@ -45,13 +46,9 @@ class ThreadPlanStepOverRange : public ThreadPlanStepRange,
 
   void SetupAvoidNoDebug(LazyBool step_out_avoids_code_without_debug_info);
   bool IsEquivalentContext(const SymbolContext &context);
-  // Clear and create a new ThreadPlanSingleThreadTimeout to detect if program
-  // is moving forward.
-  void ResetSingleThreadTimeout();
 
   bool m_first_resume;
   lldb::RunMode m_run_mode;
-  ThreadPlanSingleThreadTimeout::TimeoutInfo m_timeout_info;
 
   ThreadPlanStepOverRange(const ThreadPlanStepOverRange &) = delete;
   const ThreadPlanStepOverRange &

lldb/include/lldb/Target/TimeoutResumeAll.h

@@ -0,0 +1,40 @@
+//===-- TimeoutResumeAll.h -------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TARGET_TIMEOUTRESUMEALL_H
+#define LLDB_TARGET_TIMEOUTRESUMEALL_H
+
+#include "lldb/Target/ThreadPlanSingleThreadTimeout.h"
+
+namespace lldb_private {
+
+// Mixin class that provides capability for ThreadPlan that supports single
+// thread execution to resume all threads after a timeout.
+// Opt-in thread plan should call PushNewTimeout() in its DidPush() and
+// ResumeWithTimeout() during DoWillResume().
+class TimeoutResumeAll {
+public:
+  TimeoutResumeAll(Thread &thread) : m_thread(thread) {}
+
+  void PushNewTimeout() {
+    ThreadPlanSingleThreadTimeout::PushNewWithTimeout(m_thread, m_timeout_info);
+  }
+
+  void ResumeWithTimeout() {
+    ThreadPlanSingleThreadTimeout::ResumeFromPrevState(m_thread,
+                                                       m_timeout_info);
+  }
+
+private:
+  Thread &m_thread;
+  ThreadPlanSingleThreadTimeout::TimeoutInfo m_timeout_info;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_TIMEOUTRESUMEALL_H

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

@@ -440,11 +440,9 @@ class ProcessGDBRemote : public Process,
   void HandleStopReply() override;
   void HandleAsyncStructuredDataPacket(llvm::StringRef data) override;
 
-  // Handle thread specific async interrupt and return the original thread
-  // that requested the async interrupt. It can be null if original thread
-  // has exited.
-  lldb::ThreadSP HandleThreadAsyncInterrupt(uint8_t signo,
-                                            const std::string &description);
+  lldb::ThreadSP
+  HandleThreadAsyncInterrupt(uint8_t signo,
+                             const std::string &description) override;
 
   void SetThreadPc(const lldb::ThreadSP &thread_sp, uint64_t index);
   using ModuleCacheKey = std::pair<std::string, std::string>;

lldb/source/Target/Process.cpp

@@ -3965,7 +3965,7 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) {
 
       if (interrupt_requested) {
         if (StateIsStoppedState(internal_state, true)) {
-          // Oly mark interrupt event if it is not thread specific async
+          // Only mark interrupt event if it is not thread specific async
           // interrupt.
           if (m_interrupt_tid == LLDB_INVALID_THREAD_ID) {
             // We requested the interrupt, so mark this as such in the stop

lldb/source/Target/StopInfo.cpp

@@ -1140,15 +1140,6 @@ class StopInfoInterrupt : public StopInfo {
     return lldb::eStopReasonInterrupt;
   }
 
-  bool ShouldStopSynchronous(Event *event_ptr) override { return true; }
-
-  bool ShouldStop(Event *event_ptr) override {
-    ThreadSP thread_sp(m_thread_wp.lock());
-    if (thread_sp)
-      return thread_sp->GetProcess()->GetUnixSignals()->GetShouldStop(m_value);
-    return false;
-  }
-
   const char *GetDescription() override {
     if (m_description.empty()) {
       m_description = "async interrupt";

lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp

@@ -56,15 +56,12 @@ std::string ThreadPlanSingleThreadTimeout::StateToString(State state) {
   }
 }
 
-void ThreadPlanSingleThreadTimeout::CreateNew(Thread &thread,
-                                              TimeoutInfo &info) {
+void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread,
+                                                       TimeoutInfo &info) {
   uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout();
   if (timeout_in_ms == 0)
     return;
 
-  if (info.m_instance != nullptr)
-    return;
-
   // Do not create timeout if we are not stopping other threads.
   if (!thread.GetCurrentPlan()->StopOthers())
     return;
@@ -77,8 +74,8 @@ void ThreadPlanSingleThreadTimeout::CreateNew(Thread &thread,
   LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout pushing a brand new one");
 }
 
-void ThreadPlanSingleThreadTimeout::ResetFromPrevState(Thread &thread,
-                                                       TimeoutInfo &info) {
+void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread,
+                                                        TimeoutInfo &info) {
   uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout();
   if (timeout_in_ms == 0)
     return;

lldb/source/Target/ThreadPlanStepOverRange.cpp

@@ -37,8 +37,8 @@ ThreadPlanStepOverRange::ThreadPlanStepOverRange(
     : ThreadPlanStepRange(ThreadPlan::eKindStepOverRange,
                           "Step range stepping over", thread, range,
                           addr_context, stop_others),
-      ThreadPlanShouldStopHere(this), m_first_resume(true),
-      m_run_mode(stop_others) {
+      ThreadPlanShouldStopHere(this), TimeoutResumeAll(thread),
+      m_first_resume(true), m_run_mode(stop_others) {
   SetFlagsToDefault();
   SetupAvoidNoDebug(step_out_avoids_code_without_debug_info);
 }
@@ -346,7 +346,7 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) {
 
 void ThreadPlanStepOverRange::DidPush() {
   if (m_run_mode == lldb::eOnlyThisThread && IsControllingPlan())
-    ThreadPlanSingleThreadTimeout::CreateNew(GetThread(), m_timeout_info);
+    PushNewTimeout();
 }
 
 bool ThreadPlanStepOverRange::DoPlanExplainsStop(Event *event_ptr) {
@@ -427,7 +427,6 @@ bool ThreadPlanStepOverRange::DoWillResume(lldb::StateType resume_state,
     }
   }
   if (m_run_mode == lldb::eOnlyThisThread && IsControllingPlan())
-    ThreadPlanSingleThreadTimeout::ResetFromPrevState(GetThread(),
-                                                      m_timeout_info);
+    ResumeWithTimeout();
   return true;
 }