[Lldb-commits] [PATCH] D81838: [debugserver] wait for STDIO thread to finish before flushing output
ukalnins created this revision.
ukalnins added a reviewer: jasonmolenda.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
When process, that debugserver is attached to, exits, there is a race condition
if the process STDIO will be consumed and appened to internal stdio buffer
before
debugserver flushes the stdio buffer for the last time and sends process exit
status to lldb.
This patch adds the ability to signal the stdio thread to stop and wait for it
to finish and
uses this to wait for the thread to finish before flushing output for the last
time.
This is a potential patch for https://bugs.llvm.org/show_bug.cgi?id=45454
To reliably reproduce the problem I added usleep(1000) in
MachProcess::STDIOThread before calls to AppendSTDOUT.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D81838
Files:
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/DNB.h
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/debugserver.cpp
Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -533,6 +533,13 @@
ctx.EventsAsString(set_events, set_events_str));
if (set_events) {
+
+ if (set_events & RNBContext::event_proc_thread_exiting &&
+ remote->Context().HasValidProcessID()) {
+// Make sure we have all the STDIO read from the process before
+// flushing.
+DNBStopSTDIOThread(remote->Context().ProcessID());
+ }
if ((set_events & RNBContext::event_proc_thread_exiting) ||
(set_events & RNBContext::event_proc_stdio_available)) {
remote->FlushSTDIO();
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -1298,15 +1298,29 @@
pthread_join(m_profile_thread, NULL);
m_profile_thread = NULL;
}
+ StopSTDIOThread();
}
bool MachProcess::StartSTDIOThread() {
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s ( )", __FUNCTION__);
+ if (pipe(m_stdio_thread_interrupt) == -1) {
+return false;
+ }
// Create the thread that watches for the child STDIO
return ::pthread_create(&m_stdio_thread, NULL, MachProcess::STDIOThread,
this) == 0;
}
+void MachProcess::StopSTDIOThread() {
+ if (m_stdio_thread) {
+::write(m_stdio_thread_interrupt[1], "", 1);
+pthread_join(m_stdio_thread, nullptr);
+m_stdio_thread = 0;
+close(m_stdio_thread_interrupt[0]);
+close(m_stdio_thread_interrupt[1]);
+ }
+}
+
void MachProcess::SetEnableAsyncProfiling(bool enable, uint64_t interval_usec,
DNBProfileDataScanType scan_type) {
m_profile_enabled = enable;
@@ -2362,6 +2376,8 @@
DNBError err;
int stdout_fd = proc->GetStdoutFileDescriptor();
int stderr_fd = proc->GetStderrFileDescriptor();
+ int interrupt_fd = proc->GetStdoutThreadInterruptFileDescriptor();
+
if (stdout_fd == stderr_fd)
stderr_fd = -1;
@@ -2374,7 +2390,10 @@
FD_SET(stdout_fd, &read_fds);
if (stderr_fd >= 0)
FD_SET(stderr_fd, &read_fds);
-int nfds = std::max(stdout_fd, stderr_fd) + 1;
+
+FD_SET(interrupt_fd, &read_fds);
+
+int nfds = std::max({stdout_fd, stderr_fd, interrupt_fd}) + 1;
int num_set_fds = select(nfds, &read_fds, NULL, NULL, NULL);
DNBLogThreadedIf(LOG_PROCESS,
@@ -2452,6 +2471,9 @@
} while (bytes_read > 0);
}
+ if (FD_ISSET(interrupt_fd, &read_fds)) {
+break;
+ }
}
}
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s (%p): thread exiting...",
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.h
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -187,6 +187,7 @@
// Exception thread functions
bool StartSTDIOThread();
+ void StopSTDIOThread();
static void *STDIOThread(void *arg);
void ExceptionMessageReceived(const MachException::Message &exceptionMessage);
task_t ExceptionMessageBundleComplete();
@@ -299,6 +300,9 @@
int GetStdinFileDescriptor() const { return m_child_stdin; }
int GetStdoutFileDescriptor() const { return m_child_stdout; }
int GetStderrFileDescriptor() const { return m_child_stderr; }
+ int GetStdoutThreadInterruptFileDescriptor() const {
+return m_stdio_thread_interrupt[0];
+ }
void AppendSTDOUT(char *s, size_t len);
size_t GetAvailableSTDOUT(char *buf, size_t buf_size);
size_t GetAvailableSTDERR(char *buf, size_t buf_size);
@@ -361,6 +365,9 @@
uint32_t m_stop_count; // A count
[Lldb-commits] [PATCH] D81838: [debugserver] wait for STDIO thread to finish before flushing output
ukalnins updated this revision to Diff 270732.
ukalnins added a comment.
Fixed merge conflict.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81838/new/
https://reviews.llvm.org/D81838
Files:
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/DNB.h
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/debugserver.cpp
Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -533,6 +533,13 @@
ctx.EventsAsString(set_events, set_events_str));
if (set_events) {
+
+ if (set_events & RNBContext::event_proc_thread_exiting &&
+ remote->Context().HasValidProcessID()) {
+// Make sure we have all the STDIO read from the process before
+// flushing.
+DNBStopSTDIOThread(remote->Context().ProcessID());
+ }
if ((set_events & RNBContext::event_proc_thread_exiting) ||
(set_events & RNBContext::event_proc_stdio_available)) {
remote->FlushSTDIO();
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -1316,15 +1316,29 @@
}
m_activities.Clear();
StopProfileThread();
+ StopSTDIOThread();
}
bool MachProcess::StartSTDIOThread() {
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s ( )", __FUNCTION__);
+ if (pipe(m_stdio_thread_interrupt) == -1) {
+return false;
+ }
// Create the thread that watches for the child STDIO
return ::pthread_create(&m_stdio_thread, NULL, MachProcess::STDIOThread,
this) == 0;
}
+void MachProcess::StopSTDIOThread() {
+ if (m_stdio_thread) {
+::write(m_stdio_thread_interrupt[1], "", 1);
+pthread_join(m_stdio_thread, nullptr);
+m_stdio_thread = 0;
+close(m_stdio_thread_interrupt[0]);
+close(m_stdio_thread_interrupt[1]);
+ }
+}
+
void MachProcess::SetEnableAsyncProfiling(bool enable, uint64_t interval_usec,
DNBProfileDataScanType scan_type) {
m_profile_enabled = enable;
@@ -2388,6 +2402,8 @@
DNBError err;
int stdout_fd = proc->GetStdoutFileDescriptor();
int stderr_fd = proc->GetStderrFileDescriptor();
+ int interrupt_fd = proc->GetStdoutThreadInterruptFileDescriptor();
+
if (stdout_fd == stderr_fd)
stderr_fd = -1;
@@ -2400,7 +2416,10 @@
FD_SET(stdout_fd, &read_fds);
if (stderr_fd >= 0)
FD_SET(stderr_fd, &read_fds);
-int nfds = std::max(stdout_fd, stderr_fd) + 1;
+
+FD_SET(interrupt_fd, &read_fds);
+
+int nfds = std::max({stdout_fd, stderr_fd, interrupt_fd}) + 1;
int num_set_fds = select(nfds, &read_fds, NULL, NULL, NULL);
DNBLogThreadedIf(LOG_PROCESS,
@@ -2478,6 +2497,9 @@
} while (bytes_read > 0);
}
+ if (FD_ISSET(interrupt_fd, &read_fds)) {
+break;
+ }
}
}
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s (%p): thread exiting...",
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.h
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -187,6 +187,7 @@
// Exception thread functions
bool StartSTDIOThread();
+ void StopSTDIOThread();
static void *STDIOThread(void *arg);
void ExceptionMessageReceived(const MachException::Message &exceptionMessage);
task_t ExceptionMessageBundleComplete();
@@ -299,6 +300,9 @@
int GetStdinFileDescriptor() const { return m_child_stdin; }
int GetStdoutFileDescriptor() const { return m_child_stdout; }
int GetStderrFileDescriptor() const { return m_child_stderr; }
+ int GetStdoutThreadInterruptFileDescriptor() const {
+return m_stdio_thread_interrupt[0];
+ }
void AppendSTDOUT(char *s, size_t len);
size_t GetAvailableSTDOUT(char *buf, size_t buf_size);
size_t GetAvailableSTDERR(char *buf, size_t buf_size);
@@ -368,6 +372,9 @@
uint32_t m_stop_count; // A count of many times have we stopped
pthread_t m_stdio_thread; // Thread ID for the thread that watches for child
// process stdio
+ int m_stdio_thread_interrupt[2]; // File descriptor pair to notify STDIO
+ // thread that it should stop waiting for
+ // input and finish.
PThreadMutex m_stdio_mutex; // Multithreaded protection for stdio
std::string m_stdout_data;
Index: lldb/tools/debugserver/source/DNB.h
===
--- lldb/tools/debugserver/source/DNB.h
+++ lldb/tools/debugserver/source/DNB.h
@@ -235,4 +2
