Close source file on multipart upload_file part failure - #3409
Merged
Conversation
sichanyoo
approved these changes
Jul 22, 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
2 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.
Issue
Fixes #3408
Description
MultipartFileUploaderclosed each part's source file (p[:body].close) only on the success path, before therescue/ensure. When a part upload raised, that line was skipped, so the failed part'sFilePartwas left open.FilePartopens the source lazily on first read and only releases it in#close, so every failed part leaked a file descriptor.In long-running processes that retry failed uploads, this accumulates until file handles are exhausted. When the source is an unlinked temp file, the open descriptor also keeps its disk blocks allocated, so
/tmpcan fill (see the downstream report in mastodon/mastodon#39863).Fix
Move
p[:body].closeinto theensureblock so the file is released on both the success and error paths.FilePart#closeis a no-op when the file was never opened andIO#closeis idempotent, so there is no double-close concern.Testing
Added a regression spec asserting every
FilePartis closed when a part upload fails. It fails onmain(the failed part is closed 0 times) and passes with this change. The existing specs and related uploader specs still pass.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.