Stop opening a null-device handle per Capture and spinner task - #629
Draft
GoodForOneFare wants to merge 1 commit into
Draft
Stop opening a null-device handle per Capture and spinner task#629GoodForOneFare wants to merge 1 commit into
GoodForOneFare wants to merge 1 commit into
Conversation
GoodForOneFare
force-pushed
the
gordo-capture-fd-leak
branch
from
August 14, 2026 19:09
a377811 to
835af9e
Compare
Capture#initialize and SpinGroup#add both defaulted duplicate_output_to to a freshly opened File::NULL handle. Every capture, including one per spinner task, therefore opened an OS handle solely to discard writes. Ruby closes these handles when their File objects are finalized, but they remain open while captures or queued task closures are live. Default duplicate_output_to to nil and skip the duplicate write when unset. Type the option as io_like? so StringIO remains supported, and ignore IOError from caller-owned duplicate streams to match the module-level duplicate writer. The regression tests deterministically assert that the two defaults do not open the null device and cover both StringIO duplication and a duplicate stream closed during capture. Co-authored-by: River <river@shopify.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Assisted-By: devx/cceb8550-0aa0-4aa5-b0d0-bd55a129e07c
GoodForOneFare
force-pushed
the
gordo-capture-fd-leak
branch
from
August 14, 2026 20:45
835af9e to
2837642
Compare
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.
Bug
Capture#initializeandSpinGroup#addboth defaultedduplicate_output_to:to a freshly openedFile::NULLhandle:Every capture therefore opened an OS handle solely to discard writes, including one for every spinner task. Ruby closes these handles when their
Fileobjects are finalized, but they remain open while captures or queued task closures are live.Verified on
main: constructing 10 captures opens 10 new descriptors.Fix
Default
duplicate_output_to:toniland skip the duplicate write when unset. This avoids opening a descriptor just to discard output.The option is typed as
io_like?(IO | StringIO | nil) so its signature matches the streams accepted elsewhere in cli-ui. Explicit duplicate streams continue to receive captured stdout and remain caller-owned. AnIOErrorfrom a closed duplicate stream is ignored, matching the module-level duplicate writer.The module-level
StdoutRouter.duplicate_output_toused byCLI::UI.log_output_tois a separate mechanism and is otherwise unchanged.Tests
Capturedoes not openFile::NULLby default.SpinGroup#adddoes not openFile::NULLfor each task.StringIOduplicate receives captured stdout and is not closed by the capture.🤖 Generated with Claude Code
(posted by an LLM bot on behalf of Gord)