Repository navigation
fix: wake a parked async_http body reader instead of closing under it - #107
Merged
Merged
Conversation
TRANSPORT-7's body path: a token cancelled after delivery reaches a consumer blocked in a body read through the exchange's watcher. The watcher closed the delivered response and relied on that close to wake the parked reader. Whether it does depends on io-event's selector: io_uring's poll sees a descriptor closed under a parked fiber, epoll silently drops it from its interest set. Developer machines with liburing run URing and stayed green; hosted CI runners have no liburing, run EPoll, and the reader stayed parked until the test's own 5 s bound fired (3.3, 3.4) or, on Ruby 4.0, IO#close's deferred fiber interrupt landed later in the fixture's Thread#join. The conformance mid-body cancel hung for the same reason. ResponseBody now records the fiber parked in its native read, and the watcher's #cancel_read raises an IOError into that fiber through the scheduler -- a checkpoint interrupt, as Task#cancel and with_timeout deliver -- so the reader unwinds its wait, closes the body on its own fiber and surfaces CancelledError through the token-first classifier. With no reader parked, or one on another thread or reactor, the body is closed as before. Pinned by CancelReadTest over a native body whose close never wakes its reader (the #each and #source paths), and by the body-path cancellation test run on every selector the host has, so the EPoll case is exercised on an io_uring machine too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
mainhas been red ontest (3.3),test (3.4)andtest (4.0)since phase 8c landed, and green locally, because the local machines and the hosted runners run different io-event selectors, and they differ on exactly the property the async transport's TRANSPORT-7 body path relied on.When a token is cancelled after the response is delivered, the exchange's watcher (on the reactor thread) closed the delivered response and relied on that close to wake a consumer parked in a body read. That works only by accident of the selector:
IOError: closed stream.IO::Event::Selector::EPoll#transfer) silently drops a closed descriptor from its interest set, so the reader stays parked:with_timeoutfires. That is theExpected #<Async::TimeoutError> to not be a kind of Async::TimeoutErrorfailure atcancellation_test.rb:274.minitest_driver.rb:72).IO#close's own interrupt of the parked fiber is deferred throughFiber::Scheduler#fiber_interrupt. It lands later, at whatever suspension point that fiber reaches next: hereAsyncHTTPHoldingServer#close'sThread#join, which givesIOError: stream closed in another thread.The defect is in the adapter, not in the test: a caller's reader on EPoll hangs until its own deadline.
Evidence
IO_EVENT_SELECTOR=EPoll::274).IOError: stream closed in another threadfrom the fixture's join). These are the exact CI signatures.readpartialis not woken by another fiber'scloseunder EPoll (it times out), but is woken under URing.Fix
ResponseBodynow records the fiber parked in its native read, and the scheduler that fiber is parked under. The watcher callsResponseBody#cancel_read, which does one of two things:IOErrorinto that fiber throughFiber.scheduler.raise. This is a checkpoint interrupt, the mechanismAsync::Task#canceland#with_timeoutuse. The fiber is provably suspended inside#pull's own read at that point. The reader unwinds its wait, closes the body on its own fiber, and surfacesCancelledErrorthrough the existing token-first classifier.Nothing is ever closed under a parked fiber, so neither EPoll's silent drop nor 4.0's deferred interrupt can occur. No
Timeout.timeout,Thread#raiseorThread#killis involved.Tests
response_body_test.rbCancelReadTestuses a native body whoseclosenever wakes its reader, which models EPoll independent of the host. It pins the wake on both the#eachpath and the#sourceenumerator-fiber path (Response#body_string's route). It also pins that the close runs after the read unwound, not under it. Mutation check: with#cancel_readreduced to a plain close, both tests fail on their bound instead of hanging.cancellation_test.rbruns the TRANSPORT-7 body-path test once per selector the host has (AsyncHTTPReactor::SELECTORS, via a newreactor_on(selector)), so the EPoll case runs on an io_uring machine too.Verification
cancellation_test.rbfile, each underIO_EVENT_SELECTOR=EPoll,taskset -c 0and a CPU hog on the same core, with varied seeds: 0 failures on each of 3.3.12, 3.4.10 and 4.0.6.bundle exec rake(all 24 gates) on 4.0.6: exit 0. Honest RuboCop is clean.