fix(blob): bound the stream() incomplete-content wait so a truncated blob can't spin a worker at 100% CPU (#1454) - #1456
Merged
Merged
Conversation
…ated blob can't spin a worker at 100% CPU (#1454) FileBackedBlob.stream() → readMore() busy-spun a worker thread indefinitely when a blob file was present but truncated (its 8-byte header records a known final size larger than the bytes on disk) and the write-lock still read as held. In the bytesRead===0 branch, resumeIfWriterFinished() re-entered readMore() synchronously with no backoff and no deadline, so a read at EOF returned 0 every iteration and pegged the core. checkIfIsBeingWritten() caches its result, so a writer that died or stalled without releasing its lock pinned the spin forever. This is the prod-dyn/prod-gar GDI CPU storm; it is not addressed by #1423/#1424 (the spin branch is byte-identical 5.0.28 through 5.1.8). Add a lazily-set no-progress deadline (getBlobReadTimeout(), default 20s) and a 20ms backoff before the resume re-entry, failing fast with a retryable BlobReadError(503) once the deadline passes. The deadline is set only while no bytes are readable, so a genuinely slow but progressing in-progress write - which makes progress and resolves the pull each chunk, starting each new pull with a fresh budget - is unaffected. Also track both readMore() 20ms backoff timers via `timer` so cancel()/onError can clear a pending poll, avoiding a read on a closed/reused fd when a stream is aborted mid-wait. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a no-progress deadline and a short backoff delay when reading file-backed blobs that are present but truncated while the writer lock is still held. This prevents the worker from busy-spinning at 100% CPU by avoiding synchronous re-entry of the read loop. A corresponding unit test has been added to verify that such reads fail promptly with a 503 status code. There are no review comments to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
kriszyp
marked this pull request as ready for review
June 23, 2026 12:34
Contributor
|
Reviewed; no blockers found. |
…ead writer The lock is in-process and released on unlock() or DBHandle::close() (which reaps even expired-owner locks), so a dead/crashed writer cannot leave it held. The spin is pinned by a *live* in-progress write whose source stream stalled and never reached unlock() — the writeBlobWithStream pipeline that #1444 watchdogs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DavidCockerill
approved these changes
Jun 23, 2026
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
3 participants
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.
Fixes #1454.
Summary
FileBackedBlob.stream()→readMore()busy-spun a worker thread at ~100% CPU, indefinitely, when a blob file is present but truncated (its 8-byte header records a known final size larger than the bytes actually on disk) and the write-lock reads as held. In thebytesRead === 0branch,resumeIfWriterFinished()re-enteredreadMore()synchronously with no backoff and no deadline, so a read at EOF returned 0 every iteration.checkIfIsBeingWritten()caches its result. The lock is in-process and released onunlock()/DBHandle::close(), so a dead writer can't leave it held — the pin is a live in-progress write whose source stream stalled and never reachedunlock()(thewriteBlobWithStreampipeline that #1443/#1444 watchdogs). This PR is the complementary reader-side bound.This is the prod-dyn / prod-gar GDI CPU storm (harper-pro 5.0.28), confirmed by a 6-thread CPU profile (~73% in
readMore:fs.read~48% +Buffer.allocUnsafe(256KB)~21%). The trigger is a wedged GDI blob-replication backlog (~11–12h behind) plus a 2h blob expiration: bodies evict/truncate before the backlog ships them.Why this isn't already fixed by #1423/#1424
The spin branch is byte-for-byte identical 5.0.28 → 5.1.8. #1423's
getBlobReadTimeout()timer lives in theelse if (!resumeIfWriterFinished())branch the spin never enters (resume returnstrue). #1424's descriptor cross-check isisFullRead-gated (HTTP serving streams a range → skipped) and only fires when header ≠ descriptor (the common truncation has header == descriptor).The fix
incompleteDeadline = getBlobReadTimeout(), default 20s) + a 20ms backoff before the resume re-entry. Past the deadline, fail fast with a retryableBlobReadError(503).readMore()20ms backoff timers are now assigned totimersocancel()/onErrorcan clear a pending poll (the cross-model review caught that the new one wasn't tracked → a read on a closed/reused fd if a stream is aborted mid-wait).Where to look
resources/blob.ts— theresumeIfWriterFinished()re-entry and theincompleteDeadlineinit in thecheckIfIsBeingWritten()block.unitTests/resources/blob.test.js(Blob stream() busy-spins a worker at 100% CPU on a present-but-truncated blob whose writer lock stays held (survives #1423/#1424) #1454) — present-but-truncated blob + held lock. Verified genuine: revert the source and it hangs (30s mocha timeout); with the fix it rejects 503 in ~165ms.Open items for the reviewer
The cross-model review surfaced pre-existing latent bugs in this file, in code this PR does not change — deliberately left out of this targeted patch and tracked separately in #1457:
fdis not nulled after close (double-close on a reused fd), thestart()open-retry timer is untracked,onErrorcan fire twice if a slow async read completes after rejection, and slices read sequentially from byte 0.Bound for a 5.1.x patch.
Generated with assistance from Claude (Opus 4.8). Cross-model reviewed (Codex + Gemini + Harper-domain).