Fix file descriptor leak in CLI CSV import (CSVParser never closed)#18238
Open
PDGGK wants to merge 1 commit into
Open
Fix file descriptor leak in CLI CSV import (CSVParser never closed)#18238PDGGK wants to merge 1 commit into
PDGGK wants to merge 1 commit into
Conversation
readCsvFile returns a CSVParser that owns the underlying FileInputStream, but
the import call sites never closed it: early returns on an empty file or a bad
header leaked the descriptor immediately, and importing a large directory of
CSV files could exhaust file descriptors ("Too many open files"). Wrap the
parser in try-with-resources at each call site (ImportData, ImportDataTree,
ImportDataTable, ImportSchemaTree) so it and its FileInputStream are closed on
every exit; the record stream is fully consumed inside the block, so closing on
scope exit is safe. Add a test asserting the parser is closed after use.
Signed-off-by: Zihan Dai <99155080+PDGGK@users.noreply.github.com>
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.
Description
AbstractDataTool.readCsvFilereturns aCSVParserthat owns the underlyingFileInputStream, but the CLI import call sites (ImportData,ImportDataTree,ImportDataTable,ImportSchemaTree) never closed it. The empty-file / invalid-header early returns leaked the file descriptor immediately, and importing a directory of many CSV files could exhaust the process limit withToo many open files.This wraps the parser in a try-with-resources at each call site so it (and the
FileInputStreamit wraps) is closed on every exit path. The recordStreamis fully consumed inside the block before the method returns, so closing on scope exit is safe. A test is added asserting the parser is closed after use.This closes #18237.