Skip to content

Report sendable() from the transport, not from a missing slot (#177) - #178

Merged
EdmondDantes merged 10 commits into
mainfrom
177-try-write
Aug 20, 2026
Merged

Report sendable() from the transport, not from a missing slot (#177)#178
EdmondDantes merged 10 commits into
mainfrom
177-try-write

Conversation

@EdmondDantes

@EdmondDantes EdmondDantes commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Reduced from what this PR set out to do. tryWrite() is not here — two review passes showed it cannot be honest on HTTP/1 yet, and the reason is worth writing down.

What is left, and it is a real bug

sendable() is documented as backpressure advice. On a compressed response it answered a constant true: http_compression_maybe_install_stream_wrapper replaces stream_ops with compressing_stream_ops, which carried neither a sendable nor an is_alive slot, and a NULL slot is defined as "this transport has no queue of its own — report writable". The queue was still there, one layer down. HTTP/3 had no sendable either, for a stream that does queue. Both now answer from the transport.

Why tryWrite() was dropped

A non-blocking append needs a transport that can refuse. Threading a nonblocking flag into append_chunk works on HTTP/2, HTTP/3 and the worker path. On HTTP/1 it does not: the streaming writer is http_connection_sendsend_raw, which awaits its own uv_write, so an accepted chunk parks regardless.

Routing the non-blocking case through http_connection_send_batched — the recipe WebSocket::trySend() uses — gives a second, unordered channel on a connection whose chunked framing assumes one: a chunk body waits in out_pending_buf behind an in-flight write while mark_ended's 0\r\n\r\n goes out directly and jumps ahead of it. That corrupts the body exactly when a refusal-and-retry happens, which is the case the feature exists for. On TLS it is worse: the high-water mark counts out_pending_len, which a TLS connection never fills, so the gate never fires and the failure becomes a spurious 499 instead of a refusal.

Filed as #179: one serialized outbound path per HTTP/1 connection, which tryWrite() needs and which also removes a park from the streaming hot path. #177 stays open behind it.

Suite: 276 passed, 0 failed.

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Coverage

Total lines: 82.22% → 82.10% (-0.12 pp)

File Baseline Current Δ Touched
src/compression/http_compression_response.c 80.14% 75.68% -4.47 pp
src/core/http_connection.c 75.45% 75.26% -0.19 pp
src/core/worker_dispatch.c 78.43% 76.63% -1.80 pp
src/http1/http1_stream.c 69.57% 66.23% -3.33 pp
src/http2/http2_strategy.c 83.33% 84.13% +0.79 pp
src/http3/http3_callbacks.c 82.62% 83.17% +0.55 pp
src/http3/http3_dispatch.c 82.77% 82.77% +0.00 pp
src/http3/http3_listener.c 78.08% 76.88% -1.20 pp
src/http3/http3_static_response.c 76.37% 76.37% +0.00 pp
src/http_response.c 86.11% 85.37% -0.74 pp
src/http_server_class.c 75.07% 75.03% -0.04 pp
src/http_sse.c 85.11% 85.11% +0.00 pp
src/websocket/php_websocket.c 78.44% 78.61% +0.17 pp
src/websocket/ws_session.c 90.41% 89.85% -0.55 pp

❌ Regression in touched files (> 1.0 pp drop)

  • src/compression/http_compression_response.c dropped -4.47 pp
  • src/core/worker_dispatch.c dropped -1.80 pp
  • src/http1/http1_stream.c dropped -3.33 pp

Add [coverage-drop-ok] to a commit message in this PR to override.

@EdmondDantes EdmondDantes changed the title Add tryWrite() for a non-blocking chunk offer (#177) Report sendable() from the transport, not from a missing slot (#177) Aug 19, 2026
@EdmondDantes
EdmondDantes merged commit 7e2a7ea into main Aug 20, 2026
12 of 13 checks passed
@EdmondDantes
EdmondDantes deleted the 177-try-write branch August 20, 2026 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant