Fix resource leaks in to_zip/from_zip and incorrect dirname argument in setup.py#3135
Open
jashshah999 wants to merge 1 commit intogoogle-deepmind:mainfrom
Open
Conversation
…in setup.py - to_zip: close file handle opened for string paths using try/finally - from_zip: close file handle opened for string paths using try/finally, fixing a leak on the ValueError path when the file is not a valid zip - setup.py: pass __file__ variable instead of '__file__' string literal to os.path.dirname so that current_dir resolves correctly regardless of the working directory
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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.
Summary
Fixes three bugs in the Python bindings:
Resource leaks in
to_zipandfrom_zipWhen a string filepath is passed to
to_ziporfrom_zip, the file is opened but never explicitly closed. Infrom_zip, ifzipfile.is_zipfile()returnsFalse, theValueErroris raised before theZipFilecontext manager gets a chance to close the handle.Both functions now track whether they opened the file and use
try/finallyto ensure cleanup.Incorrect
os.path.dirnameargument insetup.pyget_long_description()passes the string literal'__file__'toos.path.dirname()instead of the variable__file__. This causescurrent_dirto resolve to an empty string rather than the directory containingsetup.py, so the function only works when the working directory happens to be the same as thesetup.pydirectory.