{{ message }}
Repository navigation
close the descriptor when fdopen fails and report ftruncate errors when preparing the clone target - #609
Merged
Conversation
…rs when preparing the clone target
Member
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.

Problem
Two error paths in
lib/zip_source_file_stdio_named.cdrop information or resources:_zip_fopen_close_on_execopens the file and then wraps the descriptor withfdopen. Whenfdopenfails, for example under memory pressure, the function returnsNULLwithout closing the descriptor. The descriptor is never released again: the caller only sees the failure.zip_fdopenhandles the same situation by closing the descriptor before returning, so the helper is missing the matching cleanup. Since the helper is used every time a named archive is opened for reading and when the clonefile path opens the temporary file, an application that retries opening archives under memory pressure accumulates descriptors.In
_zip_stdio_op_create_temp_output_cloning, whenftruncatefails while sizing the freshly created clone target, the function cleans up the temporary file but returns without setting an error code. Every other failure in the function reportsZIP_ER_TMPOPENwith the errno value. A caller ofzip_source_begin_write_cloningthat hits a real truncate failure, for instance because the filesystem rejects the file size, gets a failed call whose error state still reads as no error.Change
The descriptor is closed when
fdopenfails, and theftruncatefailure path setsZIP_ER_TMPOPENwitherrno, matching the rest of the function.Testing
Since neither failure can be triggered portably through the public API alone, I verified both with fault injection on Linux in a scratch workflow that builds current
mainand this change side by side and interposesfdopenandftruncateviaLD_PRELOAD:fdopen: onmainthe process holds 9 open descriptors where it held 5 before, one leaked per attempt; with this change the count stays at 5.zip_source_begin_write_cloningwith a failingftruncate: onmainthe call fails but reportsNo error; with this change it reportsFailure to create temporary file: Input/output error.Successful paths are unchanged: the full regress suite passes, 193 tests, and the upstream workflows are green on Linux, macOS, Windows, BSD and Solaris.