[lldb-dap] Add an RAII helper for synchronous mode (NFC)

While debugging the flakiness of the launch and attach tests, I noticed
that we have some places in lldb-dap where we put the debugger in
synchronous mode and have an early exit, that would leave the debugger
in this state. This PR introduces an RAII helper to avoid such mistakes.

Author: JDevlieghere

lldb/tools/lldb-dap/DAP.cpp

@@ -820,12 +820,12 @@ llvm::Error DAP::Disconnect(bool terminateDebuggee) {
   case lldb::eStateCrashed:
   case lldb::eStateSuspended:
   case lldb::eStateStopped:
-  case lldb::eStateRunning:
-    debugger.SetAsync(false);
+  case lldb::eStateRunning: {
+    ScopeSyncMode scope_sync_mode(debugger);
     error = terminateDebuggee ? process.Kill() : process.Detach();
-    debugger.SetAsync(true);
     break;
   }
+  }
 
   SendTerminatedEvent();
 

lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp

@@ -9,6 +9,7 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
+#include "LLDBUtils.h"
 #include "RequestHandler.h"
 #include "lldb/API/SBListener.h"
 #include "llvm/Support/FileSystem.h"
@@ -138,9 +139,11 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
   }
   if (attachCommands.empty()) {
     // No "attachCommands", just attach normally.
+
     // Disable async events so the attach will be successful when we return from
     // the launch call and the launch will happen synchronously
-    dap.debugger.SetAsync(false);
+    ScopeSyncMode scope_sync_mode(dap.debugger);
+
     if (core_file.empty()) {
       if ((pid != LLDB_INVALID_PROCESS_ID) &&
           (gdb_remote_port != invalid_port)) {
@@ -161,10 +164,9 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
         // Attach by process name or id.
         dap.target.Attach(attach_info, error);
       }
-    } else
+    } else {
       dap.target.LoadCore(core_file.data(), error);
-    // Reenable async events
-    dap.debugger.SetAsync(true);
+    }
   } else {
     // We have "attachCommands" that are a set of commands that are expected
     // to execute the commands after which a process should be created. If there

lldb/tools/lldb-dap/Handler/RequestHandler.cpp

@@ -10,6 +10,7 @@
 #include "DAP.h"
 #include "Handler/ResponseHandler.h"
 #include "JSONUtils.h"
+#include "LLDBUtils.h"
 #include "Protocol/ProtocolBase.h"
 #include "Protocol/ProtocolRequests.h"
 #include "RunInTerminal.h"
@@ -138,7 +139,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
   else
     return pid.takeError();
 
-  dap.debugger.SetAsync(false);
+  std::optional<ScopeSyncMode> scope_sync_mode(dap.debugger);
   lldb::SBError error;
   dap.target.Attach(attach_info, error);
 
@@ -162,7 +163,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
 
   // Now that the actual target is just starting (i.e. exec was just invoked),
   // we return the debugger to its async state.
-  dap.debugger.SetAsync(true);
+  scope_sync_mode.reset();
 
   // If sending the notification failed, the launcher should be dead by now and
   // the async didAttach notification should have an error message, so we
@@ -244,9 +245,8 @@ llvm::Error BaseRequestHandler::LaunchProcess(
     lldb::SBError error;
     // Disable async events so the launch will be successful when we return from
     // the launch call and the launch will happen synchronously
-    dap.debugger.SetAsync(false);
+    ScopeSyncMode scope_sync_mode(dap.debugger);
     dap.target.Launch(launch_info, error);
-    dap.debugger.SetAsync(true);
     if (error.Fail())
       return llvm::make_error<DAPError>(error.GetCString());
   } else {

lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp

@@ -9,6 +9,7 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
+#include "LLDBUtils.h"
 #include "Protocol/ProtocolRequests.h"
 #include "RequestHandler.h"
 #include "llvm/Support/JSON.h"
@@ -121,8 +122,8 @@ void RestartRequestHandler::operator()(
   // Stop the current process if necessary. The logic here is similar to
   // CommandObjectProcessLaunchOrAttach::StopProcessIfNecessary, except that
   // we don't ask the user for confirmation.
-  dap.debugger.SetAsync(false);
   if (process.IsValid()) {
+    ScopeSyncMode scope_sync_mode(dap.debugger);
     lldb::StateType state = process.GetState();
     if (state != lldb::eStateConnected) {
       process.Kill();
@@ -131,7 +132,6 @@ void RestartRequestHandler::operator()(
     // for threads of the process we are terminating.
     dap.thread_ids.clear();
   }
-  dap.debugger.SetAsync(true);
 
   // FIXME: Should we run 'preRunCommands'?
   // FIXME: Should we add a 'preRestartCommands'?

lldb/tools/lldb-dap/LLDBUtils.cpp

@@ -235,4 +235,11 @@ std::string GetStringValue(const lldb::SBStructuredData &data) {
   return str;
 }
 
+ScopeSyncMode::ScopeSyncMode(lldb::SBDebugger &debugger)
+    : m_debugger(debugger), m_async(m_debugger.GetAsync()) {
+  m_debugger.SetAsync(false);
+}
+
+ScopeSyncMode::~ScopeSyncMode() { m_debugger.SetAsync(m_async); }
+
 } // namespace lldb_dap

lldb/tools/lldb-dap/LLDBUtils.h

@@ -198,6 +198,17 @@ class TelemetryDispatcher {
   lldb::SBDebugger *debugger;
 };
 
+/// RAII utility to put the debugger temporarily  into synchronous mode.
+class ScopeSyncMode {
+public:
+  ScopeSyncMode(lldb::SBDebugger &debugger);
+  ~ScopeSyncMode();
+
+private:
+  lldb::SBDebugger &m_debugger;
+  bool m_async;
+};
+
 /// Get the stop-disassembly-display settings
 ///
 /// \param[in] debugger
@@ -207,7 +218,6 @@ class TelemetryDispatcher {
 ///     The value of the stop-disassembly-display setting
 lldb::StopDisassemblyType GetStopDisassemblyDisplay(lldb::SBDebugger &debugger);
 
-
 /// Take ownership of the stored error.
 llvm::Error ToError(const lldb::SBError &error);