Skip to content

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
jashshah999:fix/resource-leaks-and-dirname-bug
Open

Fix resource leaks in to_zip/from_zip and incorrect dirname argument in setup.py#3135
jashshah999 wants to merge 1 commit intogoogle-deepmind:mainfrom
jashshah999:fix/resource-leaks-and-dirname-bug

Conversation

@jashshah999
Copy link

Summary

Fixes three bugs in the Python bindings:

Resource leaks in to_zip and from_zip

When a string filepath is passed to to_zip or from_zip, the file is opened but never explicitly closed. In from_zip, if zipfile.is_zipfile() returns False, the ValueError is raised before the ZipFile context manager gets a chance to close the handle.

Both functions now track whether they opened the file and use try/finally to ensure cleanup.

Incorrect os.path.dirname argument in setup.py

get_long_description() passes the string literal '__file__' to os.path.dirname() instead of the variable __file__. This causes current_dir to resolve to an empty string rather than the directory containing setup.py, so the function only works when the working directory happens to be the same as the setup.py directory.

…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
@google-cla
Copy link

google-cla bot commented Feb 25, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant