THRIFT-5779: Keep TServerSocket accepting after aborted connections and descriptor exhaustion

[slachiewicz]

TServerSocket::acceptImpl() threw UNKNOWN on any accept() failure, and TServerFramework::serve() then stops accepting for good. A port scanner or nc -z aborting a connection before it is accepted (ECONNABORTED) is enough to take the server down, as THRIFT-5779 reports; so is running out of descriptors (EMFILE), and the server stays down after they free up.

Now:

• errors that belong to the one connection (ECONNABORTED, EPROTO, and on Linux EPERM and the network errors accept(2) says to treat like EAGAIN): back to poll();
• out of descriptors or memory (EMFILE, ENFILE, ENOBUFS, ENOMEM): back off from 5 ms to 1 s and retry, still interruptible;
• anything else: throws as before.

Both retried classes are logged at most once a minute, so a listening socket that fails persistently (for example EOPNOTSUPP on a non-stream socket passed in by descriptor) still shows up in the log. The EINTR limit on poll() applies per accept attempt, so signals during a long EMFILE episode do not end it.

The backoff matters on Linux, which keeps the connection queued after EMFILE, so the listening socket stays readable; macOS drops it. The tests send a byte after connecting because listen() sets TCP_DEFER_ACCEPT. Replaces #2964, which retried every error without waiting. The poll loop is only re-indented; git diff -w shows the change. The error classes and the backoff step are in TServerSocketErrors.h, namespace detail: installed with the other headers, no compatibility promise. TNonblockingServer uses TNonblockingServerSocket and is not changed.

Verified: the two descriptor-exhaustion tests → 3 failures on master, pass with the change on macOS arm64.
Verified: changing the 1 s cap or dropping ECONNABORTED from the connection errors → test_accept_backoff_doubles_up_to_one_second and test_accept_error_classes fail.

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

[Copilot]

Copilot review overview

🟡 Changes recommended

The tests introduce a portability failure and do not cover key connection-error and backoff behavior.

Review effort: Balanced
Findings: 1 · 2

Open (3)

• Guard resource-limit tests for optional sys/resource.h · New
• Add coverage for non-EMFILE connection errors · New
• Test exponential backoff growth and its one-second cap · New

What changed in this PR

Improves C++ server resilience by retrying transient accept() failures instead of terminating the server loop.

Changes:

• Classifies connection and resource-exhaustion errors.
• Adds interruptible exponential retry backoff and rate-limited logging.
• Adds descriptor-exhaustion and interruption tests.

File	Description
lib/​cpp/​src/​thrift/​transport/​TServerSocket.cpp	Implements error classification, retry, and backoff behavior.
lib/​cpp/​src/​thrift/​transport/​TServerSocket.h	Stores logging throttle state.
lib/​cpp/​test/​TServerSocketTest.cpp	Tests recovery and interruption during descriptor exhaustion.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Cross-platform socket semantics and process-wide descriptor exhaustion warrant final human validation despite targeted tests.

Review effort: Balanced
Findings: None

Resolved since last review (3)

• Guard resource-limit tests for optional sys/resource.h
• Test exponential backoff growth and its one-second cap
• Add coverage for non-EMFILE connection errors

[Copilot]

Copilot review overview

🟡 Changes recommended

Per-connection errors are logged despite the PR explicitly specifying that they should not be logged.

Review effort: Balanced
Findings: 1

Open (1)

• Avoid logging transient per-connection errors · New