Hold the shared-handle test to the upload, and cover a failed read

[EdmondDantes]

Follows #292, from two critic passes over it. Every finding below was checked against the code before acting on it; where a path is read rather than run, it says so.

A failed read had nobody to release it. io_file_read_cb notifies with the request as the result and the request's own exception beside it (ext/async libuv_reactor.c), so a failed read reaches the branch that handles a failure of the event layer and never the one that inspects the request — which means the check added there in #292 was unreachable. The failure branch left the request behind, and the reactor hands an awaited request to its awaiter and nobody else. An upload from a handle opened for writing leaks 528 bytes of request per read: a debug build reports it against libuv_reactor.c(6072), with the exception object beside it. Released in that branch now, and the unreachable check is gone.

tests/curl/071-read_error_releases_its_request.phpt covers it: the leak report goes to the output run-tests compares, so the test fails on a debug build that leaks. It skips on a release build, which prints no such report, and on Windows, where a file read is served synchronously and never reaches the branch under test.

The shared-handle test did not force the overlap it tests, and the first barrier did not either. The writer did 100 writes through the thread pool while curl was still connecting, so on a machine where the writes finished before curl subscribed the engine before the fix would have passed. The barrier added for it counted scheduler turns — and the marker printed beside it fired on every run here: an empty scheduler spins through ten thousand turns while curl is still connecting, so the writes still landed before the upload. It waits five seconds by the clock now, and prints a line the expectation does not carry when it gives up, so a barrier that stops working cannot pass quietly.

Measured with the working barrier: SIGSEGV in 5 runs of 5 against the engine before #292, 5 green runs of 5 after.

Two paths closed from reading, neither reproduced by a script. The read callback answers PAUSE while a read of its own is in flight: libcurl asks for data again whenever the transfer is unpaused, and unpausing what is received resumes what is sent with it, so a second request could take the place of the first — which would then complete as a stranger to its own state, freed by nobody, its bytes missing from the body. And a state whose handle closed under it remembers the close: a handle closed with requests still in the thread pool notifies without an exception, this state waits for its own completion, and the request curl asks for next would otherwise reach the reactor for a read on a descriptor that is gone, raising inside the reactor's own callback. Two scripts written for these two paths did not reproduce either; they enforce the invariant the rest of the fix rests on.

The engine side is php-src c63ad4096fa (true-async, merged into true-async-stable).

Left out of this PR and filed as #294: a CURLFile upload keeps its read state on the mime callback argument, out of reach of curl_async_event_stop, so its event is never cleared and its subscription never dropped — the cancelled-state handling in #292 covers the CURLOPT_READDATA state alone. Read from the code; a script for it did not reproduce either, and it wants its own reproduction before a fix. Beside it, CURL_READ_OWNS_FD is read twice and set nowhere, so both close(state->file.fd) branches are dead.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!