Sometimes a closed Socket::Shared reads indefinitely and does not close session endpoint (part two)

Ref: #101

I'm still having this issue sometimes, at least on Mac OS X (Sonoma 14.0). I'm using commit 82beeea and there doesn't seem to have been any relevant changes since then 82beeea...main

Here's an example server:

hello_debugger.cpp

[I know it's not the best code with the condition variables but it shows the issue I'm facing]

#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <condition_variable>
#include <iostream>
#include <thread>
#include <vector>

#include "dap/io.h"
#include "dap/network.h"
#include "dap/protocol.h"
#include "dap/session.h"

class Event {
 public:
  // wait() blocks until the event is fired.
  void wait();

  // fire() sets signals the event, and unblocks any calls to wait().
  void fire();

 private:
  std::mutex mutex;
  std::condition_variable cv;
  bool fired = false;
};

void Event::wait() {
  std::unique_lock<std::mutex> lock(mutex);
  cv.wait(lock, [&] { return fired; });
}

void Event::fire() {
  std::unique_lock<std::mutex> lock(mutex);
  fired = true;
  cv.notify_all();
}

Event terminate;
std::vector<std::thread> threads;

void my_handler(int s) {
  printf("Caught signal %d\n", s);
  terminate.fire();
}

int main(int argc, char** argv) {
  struct sigaction sigIntHandler;

  sigIntHandler.sa_handler = my_handler;
  sigemptyset(&sigIntHandler.sa_mask);
  sigIntHandler.sa_flags = 0;

  auto _server = dap::net::Server::create();

  auto address = "0.0.0.0";

  auto started = _server->start(
      address, 12345, [&](const std::shared_ptr<dap::ReaderWriter>& rw) {
        threads.emplace_back([=] {
          auto session = dap::Session::create();

          session->onError([&](const char* msg) {
            std::cout << "Session error: " << msg << "\n";
            terminate.fire();
          });

          session->bind(rw,
                        [&]() { std::cout << "Session endpoint closed\n"; });

          terminate.wait();
        });
      });

  if (!started) {
    std::cerr << "Failed to start DAP server.\n";
    _server.reset();
  }

  sigaction(SIGINT, &sigIntHandler, NULL);

  terminate.wait();
  std::cout << "Exiting\n";

  for (auto& thread : threads) {
    thread.join();
  }

  return 0;
}

When I run the server and nc localhost 12345 and then Ctrl+C the nc process, my CPU usage flies through the roof, and it looks like there is still an infinite read:

* thread #6, stop reason = breakpoint 3.1
  * frame #0: 0x000000010008a520 hello_debugger`dap::Socket::Shared::read(this=0x000000013ef044e8, buffer=0x0000000170142c78, bytes=15) at socket.cpp:210:13
    frame #1: 0x000000010000d678 hello_debugger`dap::ContentReader::buffer(this=0x0000000140104390, bytes=15) at content_stream.cpp:151:27
    frame #2: 0x000000010000d8b4 hello_debugger`dap::ContentReader::scan(this=0x0000000140104390, seq="Content-Length:", len=15) at content_stream.cpp:96:10
    frame #3: 0x000000010000d4c0 hello_debugger`dap::ContentReader::scan(this=0x0000000140104390, str="Content-Length:") at content_stream.cpp:107:10
    frame #4: 0x000000010000d1d8 hello_debugger`dap::ContentReader::read(this=0x0000000140104390) at content_stream.cpp:56:10

Again, the recv() call is returning 0 on the closed socket:

cppdap/src/socket.cpp

Lines 208 to 209 in 6464cd7

    
           auto len = 
        
               recv(s, reinterpret_cast<char*>(buffer), static_cast<int>(bytes), 0);

The loop comes from here, where reader.isOpen() is true, but getPayload() is empty:

cppdap/src/session.cpp

Lines 87 to 91 in 6464cd7

    
           while (reader.isOpen()) { 
        
             if (auto payload = getPayload()) { 
        
               inbox.put(std::move(payload)); 
        
             } 
        
           }

I say sometimes this issue happens because sometimes the session ClosedHandler does execute ... other times, it hangs in this loop forever. Under no circumstance is the session error handler triggered.

It look like it stems from errored(SOCKET s) not returning the correct value from getsockopt:

cppdap/src/socket.cpp

Lines 81 to 84 in 6464cd7

    
           char error = 0; 
        
           socklen_t len = sizeof(error); 
        
           getsockopt(s, SOL_SOCKET, SO_ERROR, &error, &len); 
        
           return error != 0;

-exec bt

* thread #4, stop reason = breakpoint 5.1
  * frame #0: 0x000000010008aaf8 hello_debugger`(anonymous namespace)::errored(s=4) at socket.cpp:84:10
    frame #1: 0x000000010008a2f8 hello_debugger`dap::Socket::Shared::isOpen(this=0x0000000130204348) at socket.cpp:163:36
    frame #2: 0x000000010000d0a4 hello_debugger`dap::ContentReader::isOpen(this=0x00000001302043f0) at content_stream.cpp:40:27
    frame #3: 0x0000000100052738 hello_debugger`(anonymous namespace)::Impl::startProcessingMessages(this=0x00000001302047a8)> const&)::'lambda'()::operator()() const at session.cpp:87:21

error

0 '\0'

ETA: I wouldn't say getsockopt() is returning an incorrect value, it just seems that the implementation in cppdap is not robust enough. From my quick researching, a clean closed TCP socket (one that received FIN) will not be in an error state (ie. SO_ERROR sets error to 0). When the connection is closed, the current blocking recv() call returns -1, and any subsequent calls will return 0.

cppdap/src/socket.cpp

Lines 203 to 211 in 6464cd7

    
           size_t read(void* buffer, size_t bytes) { 
        
             RLock lock(mutex); 
        
             if (s == InvalidSocket) { 
        
               return 0; 
        
             } 
        
             auto len = 
        
                 recv(s, reinterpret_cast<char*>(buffer), static_cast<int>(bytes), 0); 
        
             return (len < 0) ? 0 : len; 
        
           }

The implementation of read() completely swallows the error state of recv() returning -1, and just returns to the caller the fact that "no data has been read":

cppdap/src/socket.cpp

Lines 203 to 211 in 6464cd7

    
           size_t read(void* buffer, size_t bytes) { 
        
             RLock lock(mutex); 
        
             if (s == InvalidSocket) { 
        
               return 0; 
        
             } 
        
             auto len = 
        
                 recv(s, reinterpret_cast<char*>(buffer), static_cast<int>(bytes), 0); 
        
             return (len < 0) ? 0 : len; 
        
           }

Modifying read():

diff --git a/src/socket.cpp b/src/socket.cpp
index e9402d3..87ce424 100644
--- a/src/socket.cpp
+++ b/src/socket.cpp
@@ -207,7 +207,13 @@ class dap::Socket::Shared : public dap::ReaderWriter {
     }
     auto len =
         recv(s, reinterpret_cast<char*>(buffer), static_cast<int>(bytes), 0);
-    return (len < 0) ? 0 : len;
+
+    if (len < 0) {
+      s = InvalidSocket;
+      return 0;
+    }
+
+    return len;
   }

Seems to solve the problem... is this an acceptable solution? If so, I can create a pull request.

Navigation Menu

Repository navigation

Sometimes a closed Socket::Shared reads indefinitely and does not close session endpoint (part two) #154

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	auto len =
	recv(s, reinterpret_cast<char*>(buffer), static_cast<int>(bytes), 0);

	while (reader.isOpen()) {
	if (auto payload = getPayload()) {
	inbox.put(std::move(payload));
	}
	}

	char error = 0;
	socklen_t len = sizeof(error);
	getsockopt(s, SOL_SOCKET, SO_ERROR, &error, &len);
	return error != 0;

	size_t read(void* buffer, size_t bytes) {
	RLock lock(mutex);
	if (s == InvalidSocket) {
	return 0;
	}
	auto len =
	recv(s, reinterpret_cast<char*>(buffer), static_cast<int>(bytes), 0);
	return (len < 0) ? 0 : len;
	}

Sometimes a closed Socket::Shared reads indefinitely and does not close session endpoint (part two) #154

Description

Activity

KevinEady commented on Jun 27, 2025

KevinEady commented on Jun 27, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions