Skip to content

A cancelled coroutine leaves its buffer to a thread-pool worker #286

Closed

Description

@EdmondDantes

uv_cancel cannot stop a worker that has started, so a coroutine cancelled mid-operation returns while the worker still names the caller's memory.

Read. The coroutine leaves the loop in php_stdiop_read, libuv_io_req_dispose only marks DISPOSE_PENDING, and the pin is dropped without stream->pending_free — no close happened. The next fclose frees stream->readbuf while uv_fs_read is still writing into it. ASAN, 100 rounds of a cancelled 4 MB read: WRITE of size 4194304 from uv__fs_read in a worker thread, freed by php_stream_free.

Write. Same shape: php_stdiop_write hands the caller's buffer to uv_fs_write, and a cancelled writer's buffer — a filter bucket, a zend_string — is freed under the worker. strace of 20 rounds of a cancelled 4 MB fwrite: 18 write(...) = -1 EFAULT. EFAULT is the lucky outcome; a recycled block puts another allocation's bytes in the file.

Fixed for the thread pool

The buffer the worker names belongs to the request, not to the caller: a read lands in it and the completion copies across unless the request was abandoned, a write copies its payload at submit. Both are freed by libuv_io_req_dispose, which already defers while the operation is in flight. Nothing in php-src changes. Evidence: tests/io/099-cancel_during_io.phpt, red under ASAN before, green after.

Open: a Windows stream read

libuv_io_alloc_cb hands req->base.buf — the caller's buffer — to libuv. On Windows a console line read runs on a thread of libuv's own that uv_read_stop does not join, and an overlapped read keeps the address after the cancel; a cancelled fread on a tty, and possibly on a pipe or a socket, leaves that buffer under a foreign thread.

The fix above does not transfer. A stream read never sets ASYNC_IO_REQ_F_UV_IN_FLIGHT, so libuv_io_req_dispose frees the request's buffer in the same call rather than deferring it: the window would move from fclose to the cancel itself and get wider. Closing this needs a rendezvous for the stream read — the request kept alive until libuv reports the cancelled read — and a Windows machine to prove it on, which the current setup does not have.

Activity

  1. changed the title [-]A cancelled read leaves its buffer to a worker that is still running[/-] [+]A cancelled coroutine leaves its buffer to a thread-pool worker[/+] on Sep 9, 2026
  2. self-assigned this
    on Sep 9, 2026
  3. EdmondDantes commented on Sep 12, 2026

    @EdmondDantes
    ContributorAuthor

    The deferral above is wrong on both counts, and the hole is now reproduced.

    A rendezvous is impossible, not merely hard. For a cancelled console line read libuv never calls read_cb: uv_process_tty_read_line_req sees UV_HANDLE_CANCELLATION_PENDING and drops the result. There is no completion to keep the request alive for. Only the handle's close callback, which libuv defers until reqs_pending drains, is late enough.

    A Windows machine was not the missing piece; an instrumented libuv was. The prebuilt libuv.lib this build links is not compiled with /fsanitize=address, so a write from a libuv worker thread into freed heap is invisible to ASAN — the 3500 cancelled STDIN reads mentioned above proved nothing, and neither did 800 later ones. Rebuilt with the sanitizer, and with a Sleep(200) in uv_tty_line_read_thread between ReadConsoleW and uv_utf16_to_wtf8 to widen the race, the defect is red at once:

    ERROR: AddressSanitizer: heap-use-after-free
    WRITE of size 1 thread T-1
        #0 uv_utf16_to_wtf8 src\idna.c:504
        #1 uv_tty_line_read_thread src\win\tty.c:576
    freed by thread T0 here:
        #3 php_stream_free main\streams\streams.c:473
        #4 zif_fclose ext\standard\file.c:779
    

    The write is not a rarity. Cancellation is an injected VK_RETURN, so ReadConsoleW returns successfully every time and the worker always writes; whether it lands in freed memory is only a question of which of the two finishes first, and unpatched libuv usually wins by about a millisecond. CPU load does not shift that, since it slows both sides.

    Only the console route is exposed. A pipe (uv__pipe_read_data), a TCP socket (uv__process_tcp_read_req) and a tty in raw mode (uv__tty_queue_read_raw) all call the allocator on the loop thread once the bytes are there, so the address never outlives the callback — checked in 1.49 and 1.51.

    Known libuv limitation found on the way: uv__read_console_status is one global for the whole process, not per handle. Two console workers alive at once make a later cancellation see COMPLETED and skip the injection, and the second read then blocks until a real keystroke. Independent of this fix; worth its own issue if console reads ever overlap in practice.

    Fix in #301: a tty handle owns one 8 KB read buffer and the completion copies out of it. No automated test — a runner has no console — so the red/green procedure is written down in docs/286-windows-console-read-cancel.md.

  4. EdmondDantes commented on Sep 12, 2026

    @EdmondDantes
    ContributorAuthor

    Both halves are in: the thread-pool buffer in #287, the console read in #301. The console route has no automated test — a runner has no console — so the red/green procedure lives in docs/286-windows-console-read-cancel.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions