libstore: self-heal lost file transfer unpauses and fix wakeup races - #10
Open
devin-ai-integration[bot] wants to merge 2 commits into
Open
libstore: self-heal lost file transfer unpauses and fix wakeup races#10devin-ai-integration[bot] wants to merge 2 commits into
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
Port upstream NixOS/nix commit 0d0c333 ("libstore: Fix libcurl thread wakeup with curl >= 8.21"): track pending work in State::work under the state lock and skip the curl_multi_poll() sleep when work was queued, so a curl_multi_wakeup() consumed inside curl_multi_perform() can no longer leave the worker sleeping (up to 10s, or indefinitely if no further wakeups arrive) with an enqueued transfer or unpause request. The worker now processes incoming/unpause requests before polling instead of after. Also fix TransferItem::unpause() to clear `paused` before calling curl_easy_pause(CURLPAUSE_CONT): the CONT call synchronously flushes buffered data through the write callback, which can re-pause the transfer and set `paused = true`; the old order then overwrote it with `false`, leaving a transfer paused in curl while nix believes it is unpaused — a later unpause request becomes a no-op and the transfer never completes, matching the silent substitution hangs seen on the CI runners. Assisted-by: Devin GPT-5.2 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
A transfer paused in curl never triggers the stalled-download timeout, so a single lost unpause (whether from pause-state bookkeeping races or a missed worker wakeup) hangs the download, and any substitution waiting on it, forever. Make recovery not depend on any single unpause surviving: - download(): while the transfer may be paused, wait with a 1s bound and re-request the unpause until data flows again (dataCallback clears the flag when it delivers below-buffer-size data). Unpausing an unpaused transfer is a no-op, so retries are safe. - TransferItem::unpause(): issue CURLPAUSE_CONT unconditionally for active transfers instead of gating on our own pause flag, so a flag mismatch cannot suppress the unpause. Verified with a fault-injection harness (LD_PRELOAD shim dropping curl_easy_pause(CONT) calls): without this, one dropped unpause hangs the download indefinitely with the worker idle in curl_multi_poll and the consumer blocked on avail (matching the production CI hang); with it, downloads complete despite dropped unpauses. Assisted-by: Devin:claude-opus-4-6 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration
Bot
changed the title
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
1 participant
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.
Motivation
CI jobs on the nix runners hang silently for 90+ minutes mid-substitution ("copying path ... from cloudfront" with the NAR already downloaded and registered), e.g. monorepo run 31797505661 / job 94758301120. Root cause class: a transfer paused by nix's download-buffer backpressure whose unpause is lost never completes — a paused transfer produces no progress events, so the stalled-download timeout never fires, and the substitution goal waits forever.
Context
Reproduced against the real fork code with a fault-injection harness: an
LD_PRELOADshim that drops a singlecurl_easy_pause(CURLPAUSE_CONT)call. One lost unpause hangsFileTransfer::downloadindefinitely with the exact production shape — worker thread idle incurl_multi_poll, consumer blocked onavail, zero CPU, no timeout ever firing. A forced-pause harness also confirms curl's low-speed timeout does not fire for a paused transfer (30s+ stall on a 10s low-speed limit, transfer still "running").Fix strategy — make recovery not depend on any single unpause surviving, and remove the known ways one can be lost:
download()consumer: while the transfer may be paused, wait with a 1s bound and re-request the unpause until data flows again (dataCallbackclears the flag once it delivers data below the buffer limit). Unpausing an unpaused transfer is a no-op, so retries are safe. This self-heals any lost unpause, including causes not yet identified.TransferItem::unpause(): issueCURLPAUSE_CONTunconditionally for active transfers instead of gating on our ownpausedflag, so a flag/curl state mismatch cannot suppress the unpause; clear the flag before calling curl so a synchronous re-pause during the flush isn't clobbered.curl_multi_wakeupevents are swallowed bycurl_multi_perform(port of upstream NixOS/nix 0d0c333, curl >= 8.21 behavior; see also Really annoying apparent deadlock duringnix-store -rin Nix 1.12 NixOS/nix#1573, slow nix build in devcontainer NixOS/nix#11249, Nix build hangs with "download thread waiting for 10000 ms" loop NixOS/nix#13025).Verification:
FAULT_DROP_CONT=1,2,3,5,10): pre-fix build hangs indefinitely (stacks captured); with this PR every run completes in ~3s despite the dropped unpauses.maintainers/format.shclean; unit-test failure set identical to main baseline.Link to Devin session: https://app.devin.ai/sessions/00f9d2a60a894fb7838f60ee81bca8af
Requested by: @jld-adriano