fix: use context manager for settings file to prevent file descriptor leak#24
Open
amathxbt wants to merge 2 commits into
Open
fix: use context manager for settings file to prevent file descriptor leak#24amathxbt wants to merge 2 commits into
amathxbt wants to merge 2 commits into
Conversation
The settings file was opened without a context manager: open(settings_file, ...).read() If an exception is raised while reading (e.g. encoding error, disk issue), the file descriptor is never closed, causing a resource leak. Replacing with a 'with' statement guarantees the file is closed in all code paths.
The settings file was opened without a with-statement: open(settings_file, ...).read() If any exception is raised during reading (encoding error, disk issue, etc.), the file descriptor is never explicitly closed. CPython may close it on GC, but this is not guaranteed in all Python runtimes. Using a with-statement ensures the fd is always closed on exit.
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
In
demo/server.py, the settings file is opened without a context manager:Impact: If any exception is raised during the read (e.g. encoding error, I/O error, interrupted system call), the file descriptor is never explicitly closed. While CPython's reference-counting GC will usually close it eventually, this is not guaranteed across all Python runtimes (PyPy, Jython) and is flagged as a resource leak by static analysis tools. Under heavy load or rapid restarts this can exhaust the process's file descriptor limit.
Fix
Wrapped in a
withstatement to guarantee closure in all code paths: