[BUG] Fix transient would-block handling in the ext/http embedded server

[thc1006]

Fixes #4281
Fixes #4282
Refs #4287

Two robustness bugs in the ext/http embedded server's would-block handling, both found while reviewing #4280, plus a third that review of this PR turned up in the fix itself.

sendMore() drops the response on write backpressure (#4281)

When send() returns -1 with EWOULDBLOCK, the old code skipped the error early-return and fell through to sendBuffer.erase(0, sent); with sent == -1 the count converts to SIZE_MAX and wipes the whole unsent response. It now never erases on sent < 0, and re-registers the socket for writability on a would-block so the buffer is kept and retried.

onSocketReadable() closes on transient recv errors (#4282)

The old code closed the connection on any recv() <= 0. For a nonblocking socket, recv() == -1/EWOULDBLOCK is transient, not a close. It now closes only on an orderly shutdown (recv() == 0) or a real error, and ignores a transient would-block.

Restoring read interest after an asynchronous 100 Continue

This one came out of review of this PR, and it was introduced by the first fix above.

Reactor::addSocket() replaces the interest set rather than adding to it, so the new would-block path leaves a socket registered for writability only. Every other sending state either keeps sending or re-registers Readable once it finishes, but Sending100Continue moved to ReceivingBody without doing so and then returned to wait for a request body it could no longer be woken for. The old code avoided this by accident: it wiped the buffer on would-block, so it never reached the addSocket() call at all.

I checked the Linux half with an epoll probe registering only EPOLLOUT on a socketpair that already had readable bytes waiting:

  registered EPOLLOUT only; peer has 17 readable bytes waiting
    wait 0: n=1 events=0x4  EPOLLIN=no EPOLLOUT=yes
    wait 1: n=1 events=0x4  EPOLLIN=no EPOLLOUT=yes
    wait 2: n=1 events=0x4  EPOLLIN=no EPOLLOUT=yes

Every wait returns immediately with EPOLLOUT and never EPOLLIN, so the connection would spin without ever reading the body. On Windows there would be no further FD_WRITE and the request would simply stall.

The trigger is narrow, since it needs a 25 byte interim response to block on a fresh connection, but the interest set is now restored explicitly instead of by luck.

Error code capture ordering

Both paths read the socket error immediately after send()/recv(), before LOG_TRACE can clobber errno or the Winsock last error.

On EAGAIN vs EWOULDBLOCK

An earlier revision also checked EAGAIN, but EAGAIN and EWOULDBLOCK have the same value on every platform this repo builds on (Linux, macOS; Winsock has only WSAEWOULDBLOCK), so checking both is redundant and trips clang-tidy misc-redundant-expression. A single ErrorWouldBlock check is therefore used. POSIX does permit the two to differ, so this is a statement about the platforms here rather than a general one.

What this PR does not fix

All tracked in #4287, and I have narrowed the title so this PR does not read as a complete fix of the nonblocking write state machine:

• Windows short writes. sendMore() still performs a single send() per readiness event. Winsock permits a nonblocking send() to succeed having transferred only part of the buffer, and only guarantees a further FD_WRITE after a send has failed with WSAEWOULDBLOCK. Re-registering does not help, since addSocket() is a no-op when the flag set is unchanged. A positive short write on a large response can therefore stall. The fix is a drain loop and it deserves its own PR.
• Fatal send() errors still return true without closing the connection, removing the reactor registration or changing state.
• EINTR is not classified as retryable.
• static_cast<int>(conn.sendBuffer.size()) narrows for a handler-produced body near INT_MAX.
• SIGPIPE is [BUG] Embedded HTTP server writes can raise SIGPIPE and terminate the process #4293, with PR [BUG] Stop embedded HTTP server writes from raising SIGPIPE #4294 up. That one is worth landing early, since otherwise the process can die before any of the fatal-error handling above is reached.

On the missing coverage

Codecov reports three uncovered lines: the would-block re-register in sendMore(), the transient-would-block branch in onSocketReadable(), and the read-interest restore after Sending100Continue.

The sendMore() would-block line is now covered by ext/test/http/http_server_test.cc. I earlier argued that branch could not be reached before the drain-loop follow-up; that was wrong and I withdraw it. It is not reachable from the writable callback, since level-triggered EPOLLOUT only fires once the kernel has space and that single send() returns a positive count. It is reachable from the readable path: on a backlogged keep-alive connection whose peer has stopped reading, the kernel send buffer stays full, so the next request drives handleConnection(), then sendMore(), then send() to EWOULDBLOCK on the same call stack. The test constructs that state directly, filling a nonblocking socket's send buffer and calling sendMore(), and asserts the response is kept rather than wiped by erase(0, -1). It fails against the pre-fix header and passes against this one.

The other two lines are harder to reach without touching the header. The onSocketReadable() would-block branch is inside a private reactor callback and, under level-triggered EPOLLIN, only runs on a spurious wakeup, so reaching it needs a test seam I did not want to add to an installed header for this scoped fix. The Sending100Continue restore needs the 25 byte interim response to block on a fresh connection, which I have not found a way to force deterministically across the three CI platforms. Both are guards for transient states; if you would like them locked down as well I will add a protected seam or a gray-box handleConnection() test.

Verification

• ext/test/http/http_server_test.cc passes under ASan and UBSan, and was verified fail-before against the pre-fix header (both assertions fail there).
• clang-format 18 clean.
• clang-tidy 18 with the repo .clang-tidy on http_server.h: the same pre-existing modernize-use-emplace warnings before and after, and the fix adds none. This PR does not change warning_limit.
• The epoll behavior quoted above was measured rather than assumed.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[codecov]

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.25%. Comparing base (991ffad) to head (0061a90).

Files with missing lines	Patch %	Lines
...nclude/opentelemetry/ext/http/server/http_server.h	90.91%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4283      +/-   ##
==========================================
+ Coverage   81.24%   81.25%   +0.02%     
==========================================
  Files         446      446              
  Lines       18872    18877       +5     
==========================================
+ Hits        15330    15337       +7     
+ Misses       3542     3540       -2     

Files with missing lines	Coverage Δ
...nclude/opentelemetry/ext/http/server/http_server.h	66.09% <90.91%> (+0.18%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[thc1006]

A note on the Codecov result, since it is the first thing a reviewer will see on this PR.

The two uncovered lines are both inside the new if (sent < 0) block in sendMore(): the m_reactor.addSocket(...) re-arm and the early return true. Reaching them needs a socket that genuinely returns EWOULDBLOCK on write, which is the same property that makes testing them awkward.

There are two ways to cover it and I am fine with either:

1. A real backpressure test. Add a small setSendBufferSize() wrapper next to the existing setReuseAddr() and setTcpNoDelay() in socket_tools.h, pin SO_SNDBUF low on the accepted socket, have the client stop reading, and send a response larger than the pinned buffer. This exercises the actual lines, but it leans on kernel buffer behavior and I would want to run it a few hundred times before trusting it not to flake on the macOS and Windows runners.

2. Extract the two decisions into pure static helpers, along the lines of shouldCloseAfterRecv(int received, int error), and test those directly. That is deterministic and portable and it covers the logic that was actually wrong, but it does not touch the two lines Codecov is counting, and it changes the shape of an installed header for a testing reason.

My own preference is to leave the PR as it stands and treat the gap as known, because the uncovered part is a two line reaction to a kernel state rather than logic that can be wrong in an interesting way. I am happy to add either variant if you would rather have one, and happy to hear that a third approach is better.

One other thing worth flagging while this is open. Reading the rest of the file for this fix turned up several further problems, including what looks like a use-after-free when a handler returns -1. They are collected with line references in #4287, and the memory safety one has its own issue in #4288. None of them are in scope here, and this PR does not depend on any of them.

[thc1006]

The macOS job here is red on config.ProgrammaticConfigTest.EnabledConfigProducesProviders, and I do not think it is related to this change.

It fails as a CTest timeout rather than an assertion, and sdk/test/configuration/programmatic_configuration_test.cc contains no reference to http_server.h or anything else under ext/http, so the header this PR touches is not in that test binary. The two failing runs took 33 minutes against 9 to 12 for the ones that passed, which fits the timeout itself being what consumed those minutes.

Four of my other open PRs passed the same job today, two of them in a window that overlaps a failing run, so a bad runner period does not look like the explanation either. I have also seen this test time out on the pre-rebase revision of this PR and on the tsan job.

I cannot re-run jobs on this repository. Happy to rebase if a clean run would be more useful than my word for it, although the branch is only behind by dependabot commits.