debugger: handle read errors on POSIX pipe and test abrupt disconnect

The POSIX pipe read wrappers stored the return of ::read() into an
unsigned result, so a negative count (e.g. EBADF from a concurrent
close on another thread) became SIZE_MAX. ContentReader::buffer()
would then try to grow its deque by ~18 exabytes and crash with
either std::length_error (glibc) or a stack smash (libc++). Treat
any non-positive ::read() return as EOF/error, close the pipe, and
return 0 so the peer's SessionThread observes a clean empty payload.

cmDebuggerPipeClient is test-only infrastructure; production cmake
is always the pipe server. Production close() assumes sequential
access, which holds in normal use (the adapter destructor joins
SessionThread before closing the connection). The new abrupt-
disconnect test however needs to wake a sibling thread blocked in
read() on the same fd, which Linux ::close() does not do.

Add a ShutdownForTesting() method that calls shutdown(SHUT_RDWR)
without freeing the fd, so any concurrent blocking read wakes
with a clean zero-length return. On Windows, CloseHandle already
cancels pending overlapped I/O, so the helper just forwards to
close(). Use it from testProtocolWithPipesAbruptDisconnect in
place of close().

Source/cmDebuggerPosixPipeConnection.cxx

@@ -10,6 +10,7 @@
 #include <unistd.h>
 
 #include <sys/socket.h>
+#include <sys/types.h>
 
 namespace cmDebugger {
 
@@ -110,10 +111,13 @@ size_t cmDebuggerPipeConnection_POSIX::read(void* buffer, size_t n)
 {
   size_t result = 0;
   if (rw_pipe >= 0) {
-    result = ::read(rw_pipe, buffer, n);
-    if (result == 0) {
+    ssize_t count = ::read(rw_pipe, buffer, n);
+    if (count <= 0) {
+      // EOF or error (including EBADF from a concurrent close).
       close();
+      return 0;
     }
+    result = static_cast<size_t>(count);
   }
 
   return result;
@@ -174,17 +178,26 @@ void cmDebuggerPipeClient_POSIX::close()
   }
 }
 
+void cmDebuggerPipeClient_POSIX::ShutdownForTesting()
+{
+  if (isOpen()) {
+    ::shutdown(rw_pipe, SHUT_RDWR);
+  }
+}
+
 size_t cmDebuggerPipeClient_POSIX::read(void* buffer, size_t n)
 {
-  int count = 0;
+  ssize_t count = 0;
   if (isOpen()) {
-    count = static_cast<int>(::read(rw_pipe, buffer, n));
-    if (count == 0) {
+    count = ::read(rw_pipe, buffer, n);
+    if (count <= 0) {
+      // EOF or error (including EBADF from a concurrent close).
       close();
+      return 0;
     }
   }
 
-  return count;
+  return static_cast<size_t>(count);
 }
 
 bool cmDebuggerPipeClient_POSIX::write(void const* buffer, size_t n)

Source/cmDebuggerPosixPipeConnection.h

@@ -69,6 +69,11 @@ public:
   size_t read(void* buffer, size_t n) override;
   bool write(void const* buffer, size_t n) override;
 
+  // Unit-test helper: signal EOF on both halves of the socket without
+  // freeing the fd, so a sibling thread blocked in read() wakes up
+  // cleanly. Production code does not use cmDebuggerPipeClient.
+  void ShutdownForTesting();
+
 private:
   std::string const PipeName;
   int rw_pipe = -1;

Source/cmDebuggerWindowsPipeConnection.h

@@ -89,6 +89,11 @@ public:
   size_t read(void* buffer, size_t n) override;
   bool write(void const* buffer, size_t n) override;
 
+  // Unit-test helper: on Windows CloseHandle already cancels any
+  // pending overlapped I/O, so this just delegates to close().
+  // Production code does not use cmDebuggerPipeClient.
+  void ShutdownForTesting() { this->close(); }
+
 private:
   std::string GetErrorMessage(DWORD errorCode);
 

Tests/CMakeLib/testDebuggerAdapterPipe.cxx

@@ -186,13 +186,24 @@ bool testProtocolWithPipesAbruptDisconnect()
   std::future<void> adapterFinishedFuture =
     adapterFinishedPromise.get_future();
 
-  std::promise<void> pipeClosedPromise;
-  std::future<void> pipeClosedFuture = pipeClosedPromise.get_future();
-
   std::promise<bool> initializedEventReceivedPromise;
   std::future<bool> initializedEventReceivedFuture =
     initializedEventReceivedPromise.get_future();
 
+  std::promise<bool> exitedEventReceivedPromise;
+  std::future<bool> exitedEventReceivedFuture =
+    exitedEventReceivedPromise.get_future();
+
+  std::promise<bool> terminatedEventReceivedPromise;
+  std::future<bool> terminatedEventReceivedFuture =
+    terminatedEventReceivedPromise.get_future();
+
+  std::promise<bool> threadStartedPromise;
+  std::future<bool> threadStartedFuture = threadStartedPromise.get_future();
+
+  std::promise<bool> threadExitedPromise;
+  std::future<bool> threadExitedFuture = threadExitedPromise.get_future();
+
   auto futureTimeout = std::chrono::seconds(60);
   auto disconnectTimeout = std::chrono::seconds(10);
 
@@ -208,17 +219,26 @@ bool testProtocolWithPipesAbruptDisconnect()
   client->registerHandler([&](dap::InitializedEvent /*unused*/) {
     initializedEventReceivedPromise.set_value(true);
   });
+  client->registerHandler([&](dap::ExitedEvent /*unused*/) {
+    exitedEventReceivedPromise.set_value(true);
+  });
+  client->registerHandler([&](dap::TerminatedEvent const& /*unused*/) {
+    terminatedEventReceivedPromise.set_value(true);
+  });
+  client->registerHandler([&](dap::ThreadEvent const& e) {
+    if (e.reason == "started") {
+      threadStartedPromise.set_value(true);
+    } else if (e.reason == "exited") {
+      threadExitedPromise.set_value(true);
+    }
+  });
 
   // Raw thread (not ScopedThread): we need to be able to detach on
-  // failure so the test process can exit even if the bug is present.
-  //
-  // Note: we deliberately do NOT call ReportExitCode() here. With the
-  // bug present, an attempted write to a closed pipe would trigger the
-  // dap::Session error handler, which masks the busy-loop condition we
-  // are trying to test for. Instead we let the adapter destructor join
-  // the SessionThread directly: if SessionThread is spinning on EOF the
-  // join will hang, the adapterFinishedFuture wait below will time out,
-  // and the test will fail.
+  // failure so the test process can exit even if the regression
+  // returns. With the fix in place, ReportExitCode()'s wait on
+  // DisconnectEvent unblocks when the SessionThread detects EOF on the
+  // pipe; without the fix, it blocks forever and the adapter
+  // destructor hangs in SessionThread.join().
   std::thread debuggerThread([&]() {
     try {
       auto connection =
@@ -228,9 +248,12 @@ bool testProtocolWithPipesAbruptDisconnect()
       std::shared_ptr<cmDebugger::cmDebuggerAdapter> debuggerAdapter =
         std::make_shared<cmDebugger::cmDebuggerAdapter>(
           connection, dap::file(stdout, false));
-      // Hold the adapter until the test signals that it has closed the
-      // client side of the pipe.
-      pipeClosedFuture.wait();
+      // Sends thread-exited / exited / terminated events and then
+      // blocks on DisconnectEvent. The test closes the client side of
+      // the pipe instead of sending a DisconnectRequest, so the only
+      // thing that will unblock this wait is the SessionThread's EOF
+      // handling in cmDebuggerAdapter.
+      debuggerAdapter->ReportExitCode(0);
       // Adapter destructed here; joins SessionThread.
     } catch (std::runtime_error const&) {
       // Swallowed: connection failures shouldn't hang the test.
@@ -249,7 +272,7 @@ bool testProtocolWithPipesAbruptDisconnect()
   client->bind(client2Debugger, client2Debugger);
 
   // Drive the full handshake so that the debugger SessionThread is up
-  // and blocked reading the pipe.
+  // and ReportExitCode is blocked on DisconnectEvent.
   dap::CMakeInitializeRequest initializeRequest;
   auto initializeResponse = client->send(initializeRequest).get();
   ASSERT_TRUE(!initializeResponse.error);
@@ -265,20 +288,33 @@ bool testProtocolWithPipesAbruptDisconnect()
 
   ASSERT_TRUE(initializedEventReceivedFuture.wait_for(futureTimeout) ==
               std::future_status::ready);
+  ASSERT_TRUE(terminatedEventReceivedFuture.wait_for(futureTimeout) ==
+              std::future_status::ready);
+  ASSERT_TRUE(threadStartedFuture.wait_for(futureTimeout) ==
+              std::future_status::ready);
+  ASSERT_TRUE(threadExitedFuture.wait_for(futureTimeout) ==
+              std::future_status::ready);
+  ASSERT_TRUE(exitedEventReceivedFuture.wait_for(futureTimeout) ==
+              std::future_status::ready);
 
   // Abruptly close the client side without sending DisconnectRequest.
   // Regression check for the busy-loop bug: the debugger adapter must
   // detect EOF on the pipe and shut down on its own.
-  client2Debugger->close();
-  pipeClosedPromise.set_value();
+  //
+  // Use ShutdownForTesting() rather than close(). The client dap::Session
+  // has its own recvThread still blocked in read() on this same socket;
+  // on Linux, ::close() on an fd does not wake a sibling thread's
+  // in-flight ::read(), which would deadlock the test. ShutdownForTesting
+  // calls shutdown(SHUT_RDWR) to signal EOF on the socket endpoint
+  // without freeing the fd, so both reads wake up naturally.
+  client2Debugger->ShutdownForTesting();
 
   bool finishedInTime = adapterFinishedFuture.wait_for(disconnectTimeout) ==
     std::future_status::ready;
   if (!finishedInTime) {
-    // Bug reproduced: the SessionThread is spinning on EOF and the
-    // adapter destructor is blocked in SessionThread.join(). Detach so
-    // the test process can exit instead of hanging in std::thread's
-    // destructor.
+    // Bug reproduced: the SessionThread is spinning on EOF and
+    // ReportExitCode is blocked on DisconnectEvent. Detach so the test
+    // process can exit instead of hanging in std::thread's destructor.
     debuggerThread.detach();
     ASSERT_TRUE(finishedInTime);
   }