Close a worker's request body by its resource, not by its address - #306

The defect

A worker's request has no resource list of its own — the one it uses belongs to the worker and is destroyed once, at teardown — so the end of a request closes the buffered request body by hand. It did that by the address kept in SG(request_info).request_body, and that address stops being the body's the moment the application closes it.

The body is a registered stream like any other. get_resources('stream') hands it to a script with a reference of its own, and fclose() on it works exactly as it does on a handle the script opened itself. That is not an exotic thing for an application to do: enumerating open streams is the ordinary way to hunt a handle leak, and closing what looks like a stray one is the next step.

What follows is invisible for one request and destructive for the next:

• fclose() frees the php_stream and leaves SG(request_info).request_body naming the block. PHP marks the resource closed and keeps its slot (PHP_STREAM_FREE_KEEP_RSRC).
• The end of the request closes the same address again. With the block still unreused, the second release lands in _php_stream_free()'s in_free guard, which reads freed memory and returns quietly — so a request that closes its body and stops shows no symptom at all.
• Once the block has been handed to something else, the end of the request closes that stream instead. It is one the script opened and is still holding, and the next request on the worker finds it destroyed. A second such round aborts the process with zend_mm_heap corrupted, taking every request the worker was serving with it.

Checking liveness by address does not answer this, and that was measured rather than assumed: looking for an entry in EG(regular_list) whose ptr equals the body finds the new stream at the same address and reports the body as live. Identity by address is not identity.

Traditional, framework and SPA serving were never affected — there the destruction of the request's resource list releases the body, and it skips what is already closed.

The fix

Ownership moves from the address to the body's zend_resource, held with a reference of our own (GC_ADDREF, given back with zend_list_delete). The close is gated on that resource still naming a stream:

res->type == php_file_le_stream() && res->ptr != NULL

True for a body nobody touched, false for one userland closed — fclose() and pclose() alike, since both go through zend_list_close() and leave type = -1, ptr = NULL. Address reuse cannot fool it either: the reference keeps the zend_resource and its number from being recycled underneath the test.

The claim is taken at every point where a body can appear without userland having run since:

1. the per-request input rebuild, which parses the body itself;
2. a wrapper over sapi_module.read_post — everything bytes reach, including a second parse from userland request_parse_body();
3. a wrapper over the php:// opener — the body php://input creates for a request that had none.

The third is not there for completeness. A multipart POST has no body at all until php://input is asked for: rfc1867 has already consumed the request and SG(post_read) is set, so the stream the wrapper makes is never read through the SAPI. Without that capture point every such request would leave a stream pair standing for the life of the worker.

Two details worth calling out for review:

• Order inside the claim. Nothing is read through SG(request_info).request_body until the claim already held confirms the address is still the body's, and when that claim is dead the step returns rather than dereferencing a pointer it knows to be stale. Without that order, the first php://temp opened after a closed body is adopted as the body and closed by the end of the request — the same defect, produced by the code fixing it.
• How the php:// wrapper is installed. The registry's own slot is swapped (zend_hash_str_find on the global wrapper table, then the pointer), not unregistered and registered again. Registration appends, which moves php:// to the end of the table and changes the order stream_get_wrappers() reports, in every mode including the ones this change has nothing to do with. Measured: …,phar,php instead of …,php,file,….

A fiber parks and restores the claim alongside the body pointer, and a fiber destroyed while parked gives it back — otherwise the zend_resource would outlive the list holding it.

Deliberately not changed

• The orphaned first body of a double request_parse_body() keeps today's behaviour: the claim moves to the new body, the old one is released but not closed. Closing it needs the php://input wrapper walk extended to it, which is a contract of its own.
• Closing the enclosed memory stream of the body's temp stream leaves ts->innerstream dangling. That reproduces in PHP-FPM as well, where the destruction of the resource list frees it a second time — upstream, not ours.
• fopen('php://input') after the body has been closed dereferences a dangling pointer inside php-src itself (php_stream_rewind(input->body)). Same story.

Verification

Two tests in the hooks profile (PHP_WORKERS=1), each red on a build missing the matching half of the fix:

• hooks/test_body_stream_not_double_closed — a request closes its own body, opens twelve php://temp in its place and parks them; the next request asks whether the resource list still holds every resource they registered. On the unfixed build a pair comes back destroyed, deterministically (retired_resource_ids: [7, 8], then [43, 44, 61, 62] on a second round), and a second round on the same worker aborts with zend_mm_heap corrupted / SIGABRT. On the fixed build: three consecutive rounds green, container log empty. The question goes to the resource list rather than to the parked handles on purpose — a parked handle carries the only reference to its resource (php_stream_to_zval hands it over without one), so the broken build frees it and even is_resource() would read freed memory.
• hooks/test_multipart_input_streams_bounded — three multipart POSTs, open-stream count flat across them. This is the only thing that fails if the php:// opener stops being a capture point.

stream_get_wrappers() order checked against a build without the hook: identical (https,ftps,compress.zlib,php,file,glob,data,http,ftp,phar).

Profiles: hooks 54/0/0, default 242/0/0, fibers 32/0/0, hooksclassic 9/0/0, worker 55/0/1 — the one error is the known shared/test_channel_async_usevar_stress flake, unrelated to request bodies.

Host: cargo fmt -- --check, cargo clippy --no-default-features -- -D warnings, cargo test --no-default-features (27 binaries) all clean.