Skip to content

Mark the handle failed when a stream write is refused at submit - #315

Merged
EdmondDantes merged 1 commit into
mainfrom
fix/refused-write-verdict
Oct 9, 2026
Merged

EdmondDantes merged 1 commit into
mainfrom
fix/refused-write-verdict

Conversation

@EdmondDantes

Copy link
Copy Markdown
Contributor

A fire-and-forget write's completion carries no status, so its free_cb reads the verdict from ZEND_ASYNC_IO_WRITE_FAILED on the handle. A refused submit reaches that free_cb from inside the submit call, through the request's dispose, and every refusal left the flag clear: a write uv_write refused at once, one over the 0x7ffff000-byte cap or the platform's buffer limit, and a vectored write with more than 65535 slots, unsupported flags or on a closed or non-stream handle. true-async-server read each of them as a write that went out and counted it in tls_bytes_ciphertext_out_total. Every refusal now sets the flag before the buffer is handed back.

A write on a closed handle returned NULL and never handed its buffer back, while writev on one did, so a fire-and-forget caller leaked it. It now goes through free_cb as on every other refusal. The server never writes to an io it has closed: its destroy clears conn->io before the close.

Part of true-async/server#396 (stage S4.1 in its dev/PLAN.md). Reviewed on the diff by a Critic twice; the second pass found nothing blocking.

Evidence:

  • sapi/asynctest/tests/test_write_refused.c (true-async/php-src, branch asynctest/refused-write-verdict): 7 of 8 cases fail on main (c95dcf7), 8 of 8 pass here, 3 runs. The eighth is the control: a write that succeeds leaves the flag clear. Without the two flags at the uv_write errors, the two cases that write to a read-only pipe fail.
  • php-async phpt: 1215 passed, 6 failed; the same 6 (curl 063, 064, 070, 071, 072, io 082) fail with this change reverted on this machine.

Not in this PR, recorded in the server plan (items 41, 42): a file write whose dispatch fails, and the Windows synchronous stream path, still never call free_cb; refusals throw before free_cb runs; no CI job runs the asynctest SAPI.

@codecov

codecov Bot commented Oct 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@EdmondDantes
EdmondDantes merged commit 8565e2c into main Oct 9, 2026
9 checks passed
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