Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1495d13

Browse filesBrowse files
JDevlieghereJDevlieghere
authored
[lldb-dap] Add an RAII helper for synchronous mode (NFC) (llvm#137900)
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.
1 parent af45da1 commit 1495d13
Copy full SHA for 1495d13

File tree

Expand file treeCollapse file tree

6 files changed

+33
-14
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+33
-14
lines changed

‎lldb/tools/lldb-dap/DAP.cpp

Copy file name to clipboardExpand all lines: lldb/tools/lldb-dap/DAP.cpp
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -820,12 +820,12 @@ llvm::Error DAP::Disconnect(bool terminateDebuggee) {
820820
case lldb::eStateCrashed:
821821
case lldb::eStateSuspended:
822822
case lldb::eStateStopped:
823-
case lldb::eStateRunning:
824-
debugger.SetAsync(false);
823+
case lldb::eStateRunning: {
824+
ScopeSyncMode scope_sync_mode(debugger);
825825
error = terminateDebuggee ? process.Kill() : process.Detach();
826-
debugger.SetAsync(true);
827826
break;
828827
}
828+
}
829829

830830
SendTerminatedEvent();
831831

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

Copy file name to clipboardExpand all lines: lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
+6-4Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "DAP.h"
1010
#include "EventHelper.h"
1111
#include "JSONUtils.h"
12+
#include "LLDBUtils.h"
1213
#include "RequestHandler.h"
1314
#include "lldb/API/SBListener.h"
1415
#include "llvm/Support/FileSystem.h"
@@ -138,9 +139,11 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
138139
}
139140
if (attachCommands.empty()) {
140141
// No "attachCommands", just attach normally.
142+
141143
// Disable async events so the attach will be successful when we return from
142144
// the launch call and the launch will happen synchronously
143-
dap.debugger.SetAsync(false);
145+
ScopeSyncMode scope_sync_mode(dap.debugger);
146+
144147
if (core_file.empty()) {
145148
if ((pid != LLDB_INVALID_PROCESS_ID) &&
146149
(gdb_remote_port != invalid_port)) {
@@ -161,10 +164,9 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
161164
// Attach by process name or id.
162165
dap.target.Attach(attach_info, error);
163166
}
164-
} else
167+
} else {
165168
dap.target.LoadCore(core_file.data(), error);
166-
// Reenable async events
167-
dap.debugger.SetAsync(true);
169+
}
168170
} else {
169171
// We have "attachCommands" that are a set of commands that are expected
170172
// to execute the commands after which a process should be created. If there

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

Copy file name to clipboardExpand all lines: lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "DAP.h"
1111
#include "Handler/ResponseHandler.h"
1212
#include "JSONUtils.h"
13+
#include "LLDBUtils.h"
1314
#include "Protocol/ProtocolBase.h"
1415
#include "Protocol/ProtocolRequests.h"
1516
#include "RunInTerminal.h"
@@ -138,7 +139,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
138139
else
139140
return pid.takeError();
140141

141-
dap.debugger.SetAsync(false);
142+
std::optional<ScopeSyncMode> scope_sync_mode(dap.debugger);
142143
lldb::SBError error;
143144
dap.target.Attach(attach_info, error);
144145

@@ -162,7 +163,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
162163

163164
// Now that the actual target is just starting (i.e. exec was just invoked),
164165
// we return the debugger to its async state.
165-
dap.debugger.SetAsync(true);
166+
scope_sync_mode.reset();
166167

167168
// If sending the notification failed, the launcher should be dead by now and
168169
// the async didAttach notification should have an error message, so we
@@ -244,9 +245,8 @@ llvm::Error BaseRequestHandler::LaunchProcess(
244245
lldb::SBError error;
245246
// Disable async events so the launch will be successful when we return from
246247
// the launch call and the launch will happen synchronously
247-
dap.debugger.SetAsync(false);
248+
ScopeSyncMode scope_sync_mode(dap.debugger);
248249
dap.target.Launch(launch_info, error);
249-
dap.debugger.SetAsync(true);
250250
if (error.Fail())
251251
return llvm::make_error<DAPError>(error.GetCString());
252252
} else {

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

Copy file name to clipboardExpand all lines: lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "DAP.h"
1010
#include "EventHelper.h"
1111
#include "JSONUtils.h"
12+
#include "LLDBUtils.h"
1213
#include "Protocol/ProtocolRequests.h"
1314
#include "RequestHandler.h"
1415
#include "llvm/Support/JSON.h"
@@ -121,8 +122,8 @@ void RestartRequestHandler::operator()(
121122
// Stop the current process if necessary. The logic here is similar to
122123
// CommandObjectProcessLaunchOrAttach::StopProcessIfNecessary, except that
123124
// we don't ask the user for confirmation.
124-
dap.debugger.SetAsync(false);
125125
if (process.IsValid()) {
126+
ScopeSyncMode scope_sync_mode(dap.debugger);
126127
lldb::StateType state = process.GetState();
127128
if (state != lldb::eStateConnected) {
128129
process.Kill();
@@ -131,7 +132,6 @@ void RestartRequestHandler::operator()(
131132
// for threads of the process we are terminating.
132133
dap.thread_ids.clear();
133134
}
134-
dap.debugger.SetAsync(true);
135135

136136
// FIXME: Should we run 'preRunCommands'?
137137
// FIXME: Should we add a 'preRestartCommands'?

‎lldb/tools/lldb-dap/LLDBUtils.cpp

Copy file name to clipboardExpand all lines: lldb/tools/lldb-dap/LLDBUtils.cpp
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,4 +235,11 @@ std::string GetStringValue(const lldb::SBStructuredData &data) {
235235
return str;
236236
}
237237

238+
ScopeSyncMode::ScopeSyncMode(lldb::SBDebugger &debugger)
239+
: m_debugger(debugger), m_async(m_debugger.GetAsync()) {
240+
m_debugger.SetAsync(false);
241+
}
242+
243+
ScopeSyncMode::~ScopeSyncMode() { m_debugger.SetAsync(m_async); }
244+
238245
} // namespace lldb_dap

‎lldb/tools/lldb-dap/LLDBUtils.h

Copy file name to clipboardExpand all lines: lldb/tools/lldb-dap/LLDBUtils.h
+11-1Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,17 @@ class TelemetryDispatcher {
198198
lldb::SBDebugger *debugger;
199199
};
200200

201+
/// RAII utility to put the debugger temporarily into synchronous mode.
202+
class ScopeSyncMode {
203+
public:
204+
ScopeSyncMode(lldb::SBDebugger &debugger);
205+
~ScopeSyncMode();
206+
207+
private:
208+
lldb::SBDebugger &m_debugger;
209+
bool m_async;
210+
};
211+
201212
/// Get the stop-disassembly-display settings
202213
///
203214
/// \param[in] debugger
@@ -207,7 +218,6 @@ class TelemetryDispatcher {
207218
/// The value of the stop-disassembly-display setting
208219
lldb::StopDisassemblyType GetStopDisassemblyDisplay(lldb::SBDebugger &debugger);
209220

210-
211221
/// Take ownership of the stored error.
212222
llvm::Error ToError(const lldb::SBError &error);
213223

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.