[debugserver] wait for STDIO thread to finish before flushing output

[ukalnins]

Summary

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.

[ukalnins]

Fixed merge conflict.

lldb/tools/debugserver/source/DNB.h

 
 // Other static nub information calls.
 const char *DNBStateAsString(nub_state_t state);
 nub_bool_t DNBResolveExecutablePath(const char *path, char *resolved_path,
                                     size_t resolved_path_size);
 bool DNBGetOSVersionNumbers(uint64_t *major, uint64_t *minor, uint64_t *patch);
 /// \return the iOSSupportVersion of the host OS.
 std::string DNBGetMacCatalystVersionString();
+
+nub_bool_t DNBStopSTDIOThread(nub_process_t pid);
+
 #endif

lldb/tools/debugserver/source/DNB.cpp

if (arch && arch[0]) {
     else if (strstr(arch, "arm64") == arch || strstr(arch, "armv8") == arch ||
              strstr(arch, "aarch64") == arch)
       return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM64);
     else if (strstr(arch, "arm") == arch)
       return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM);
   }
   return false;
 }
+
+nub_bool_t DNBStopSTDIOThread(nub_process_t pid) {
+  MachProcessSP procSP;
+  if (GetProcessSP(pid, procSP)) {
+    procSP->StopSTDIOThread();
+    return true;
+  }
+  return false;
+}

lldb/tools/debugserver/source/MacOSX/MachProcess.h

#endif // WITH_SPRINGBOARD
   void DisableAllWatchpoints(bool remove);
   bool EnableWatchpoint(nub_addr_t addr);
   uint32_t GetNumSupportedHardwareWatchpoints() const;
   DNBBreakpointList &Watchpoints() { return m_watchpoints; }
   const DNBBreakpointList &Watchpoints() const { return m_watchpoints; }
 
   // Exception thread functions
   bool StartSTDIOThread();
+  void StopSTDIOThread();
   static void *STDIOThread(void *arg);
   void ExceptionMessageReceived(const MachException::Message &exceptionMessage);
   task_t ExceptionMessageBundleComplete();
   void SharedLibrariesUpdated();
   nub_size_t CopyImageInfos(struct DNBExecutableImageInfo **image_infos,
                             bool only_changed);
 
   // Profile functions
void SetChildFileDescriptors(int stdin_fileno, int stdout_fileno,
     m_child_stdin = stdin_fileno;
     m_child_stdout = stdout_fileno;
     m_child_stderr = stderr_fileno;
   }
 
   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);
   void CloseChildFileDescriptors() {
     if (m_child_stdin >= 0) {
       ::close(m_child_stdin);
       m_child_stdin = -1;
     }
std::vector<std::string>
       m_args;              // The arguments with which the process was lauched
   int m_exit_status;       // The exit status for the process
   std::string m_exit_info; // Any extra info that we may have about the exit
   MachTask m_task;         // The mach task for this process
   uint32_t m_flags;      // Process specific flags (see eMachProcessFlags enums)
   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;
 
   bool m_profile_enabled; // A flag to indicate if profiling is enabled
   useconds_t m_profile_interval_usec; // If enable, the profiling interval in
                                       // microseconds
   DNBProfileDataScanType
       m_profile_scan_type; // Indicates what needs to be profiled

lldb/tools/debugserver/source/MacOSX/MachProcess.mm

void MachProcess::Clear(bool detaching) {
   m_stop_count = 0;
   m_thread_list.Clear();
   {
     PTHREAD_MUTEX_LOCKER(locker, m_exception_messages_mutex);
     m_exception_messages.clear();
   }
   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;
   m_profile_interval_usec = static_cast<useconds_t>(interval_usec);
   m_profile_scan_type = scan_type;
 
   if (m_profile_enabled && (m_profile_thread == NULL)) {
     StartProfileThread();
#endif
   // exception, we will get a timeout which enables us to then notify
   // our main thread that we have an exception bundle available. We then wait
   // for the main thread to tell this exception thread to start trying to get
   // exceptions messages again and we start again with a mach_msg read with
   // infinite timeout.
   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;
 
   while (stdout_fd >= 0 || stderr_fd >= 0) {
     ::pthread_testcancel();
 
     fd_set read_fds;
     FD_ZERO(&read_fds);
     if (stdout_fd >= 0)
       FD_SET(stdout_fd, &read_fds);
     if (stderr_fd >= 0)
       FD_SET(stderr_fd, &read_fds);
-    int nfds = std::max<int>(stdout_fd, stderr_fd) + 1;
+
+    FD_SET(interrupt_fd, &read_fds);
+
+    int nfds = std::max<int>({stdout_fd, stderr_fd, interrupt_fd}) + 1;
 
     int num_set_fds = select(nfds, &read_fds, NULL, NULL, NULL);
     DNBLogThreadedIf(LOG_PROCESS,
                      "select (nfds, &read_fds, NULL, NULL, NULL) => %d",
                      num_set_fds);
 
     if (num_set_fds < 0) {
       int select_errno = errno;
if (num_set_fds < 0) {
                 bytes_read);
             stderr_fd = -1;
           } else if (bytes_read > 0) {
             proc->AppendSTDOUT(s, bytes_read);
           }
 
         } while (bytes_read > 0);
       }
+      if (FD_ISSET(interrupt_fd, &read_fds)) {
+        break;
+      }
     }
   }
   DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s (%p): thread exiting...",
                    __FUNCTION__, arg);
   return NULL;
 }
 
 void MachProcess::SignalAsyncProfileData(const char *info) {

lldb/tools/debugserver/source/debugserver.cpp

DNBLogThreadedIf(LOG_RNB_EVENTS,
                      __FUNCTION__, event_mask);
     nub_event_t set_events = ctx.Events().WaitForSetEvents(event_mask);
     DNBLogThreadedIf(LOG_RNB_EVENTS,
                      "%s ctx.Events().WaitForSetEvents(0x%08x) => 0x%08x (%s)",
                      __FUNCTION__, event_mask, set_events,
                      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();
       }
 
       if (set_events & RNBContext::event_proc_profile_data) {
         remote->SendAsyncProfileData();
       }