Accept() closes the listener socket on success — a listener can only accept one connection

[cdgraff]

Summary

NodeSRT::Accept() (in src/node-srt.cc) calls srt_close() on the listener socket on the success path, so the listening socket is destroyed after every accepted connection. This makes it impossible to accept more than one connection from a single listener without re-creating it.

Details

Napi::Value NodeSRT::Accept(const Napi::CallbackInfo& info) {
  ...
  int their_fd = srt_accept(socketValue, (struct sockaddr *)&their_addr, &addr_size);
  if (their_fd == SRT_INVALID_SOCK) {
    srt_close(socketValue);      // failure path
    ...
    return Napi::Number::New(env, SRT_ERROR);
  }
  srt_close(socketValue);        // <-- success path: closes the LISTENER
  socketValue = Napi::Number::New(env, SRT_INVALID_SOCK);
  return Napi::Number::New(env, their_fd);
}

In libsrt, srt_accept() returns the newly accepted socket and leaves the listener open so it can keep accepting. Closing the listener here diverges from that contract.

Impact

• A server can accept only one connection per listener; after the first accept() the listener goes to SRT_SOCKSTATUS BROKEN and subsequent callers time out.
• To work around it, apps must re-create (bind+listen) the listener after every accept. That drops the listener's backlog of pending handshakes, so when several callers connect at nearly the same time, only one is accepted and the rest fail with an I/O error (their handshakes are lost when the listener is destroyed). This is very visible under a burst of simultaneous connections (e.g. after a restart), where the large majority of callers fail to connect.

Reproduction

1. Create a listener (createSocket → bind → listen → epollAddUsock).
2. Connect one caller; call accept(listenerFd).
3. Check getSockState(listenerFd) immediately after: it is BROKEN (6), then NONEXIST (9). Any further caller times out.

With the success-path srt_close removed, the listener stays LISTENING (3) and accepts multiple connections normally (verified: 1/3 → 3/3 sequential callers; and a burst of N simultaneous callers goes from ~1/N to N/N).

Suggested fix

Remove the srt_close(socketValue) (and the following no-op reassignment) on the success path, so Accept() only returns the accepted fd and leaves the listener open — matching libsrt semantics. The caller is responsible for closing the listener when done.

Happy to open a PR if that helps.

[birme]

Thanks for the detailed and well-diagnosed report, @cdgraff — this is exactly the kind of write-up that makes a fix easy to act on. You're right: in src/node-srt.cc, NodeSRT::Accept() calls srt_close(socketValue) on the success path, which closes the listener socket rather than leaving it open. That diverges from libsrt semantics, where srt_accept() returns the newly accepted socket and the listener stays open to keep accepting — so as you note, a listener ends up usable for only one connection (going BROKEN → NONEXIST), and the re-bind/re-listen workaround drops the pending-handshake backlog, causing most callers in a simultaneous burst to fail.

The suggested fix looks correct to us: remove the success-path srt_close(socketValue) (and the following no-op reassignment) so Accept() only returns the accepted fd and leaves the listener open, with the caller responsible for closing the listener when done. One thing we'd like a PR to include is a regression test under spec/ (or integration-tests/) that accepts multiple sequential connections from a single listener and asserts the listener stays LISTENING, covering both the sync (SRT) and async (AsyncSRT) paths.

A PR would be very welcome — please branch off master and reference this issue. Thanks again for offering to open one! We've flagged this for a maintainer to review the addon change.

Triaged by the node-srt community triage bot — confirmed bug in the native addon (Tier B); a fix PR will be reviewed and merged by a human maintainer.