-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb-dap] Fix flaky test #145231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb-dap] Fix flaky test #145231
Conversation
|
@llvm/pr-subscribers-lldb Author: None (DrSergei) ChangesThis patch fixes a possible data race between main and event handler threads. Terminated event can be sent from This patch moved from 145010 and based on idea from this comment. Full diff: https://github.com/llvm/llvm-project/pull/145231.diff 2 Files Affected:
diff --git a/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py b/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py
index ed17044a220d4..116bc53a2dde0 100644
--- a/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py
+++ b/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py
@@ -101,7 +101,8 @@ def test_server_interrupt(self):
# Interrupt the server which should disconnect all clients.
process.send_signal(signal.SIGINT)
- self.dap_server.wait_for_terminated()
+ self.dap_server.wait_for_event(["terminated", "exited"])
+ self.dap_server.wait_for_event(["terminated", "exited"])
self.assertIsNotNone(
self.dap_server.exit_status,
"Process exited before interrupting lldb-dap server",
diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
index 9b9de5e21a742..9de6283319bb6 100644
--- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
@@ -310,7 +310,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
"DAP session (" + client_name +
") error: ");
}
-
+ io->Close();
DAP_LOG(log, "({0}) client disconnected", client_name);
std::unique_lock<std::mutex> lock(dap_sessions_mutex);
dap_sessions.erase(io.get());
@@ -342,8 +342,6 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
<< " disconnected failed: "
<< llvm::toString(std::move(error)) << "\n";
}
- // Close the socket to ensure the DAP::Loop read finishes.
- sock->Close();
}
}
|
This patch fixes a possible data race between main and event handler threads. Terminated event can be sent from
Disconnectfunction or event handler. Consequently, there are some possible sequences of events. We must check events twice, because without getting an exited event,exit_statuswill be None. But, we don't know the order of events (for example, we can get terminated event before exited event), so we check events by filter. It is correct, because terminated event will be sent only once (guarded byllvm::call_once).This patch moved from 145010 and based on idea from this comment.