Skip to content

Emit ResourceWarning when GzipFile is deleted with unwritten data #130806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
cmaloney opened this issue Mar 3, 2025 · 2 comments
Closed

Emit ResourceWarning when GzipFile is deleted with unwritten data #130806

cmaloney opened this issue Mar 3, 2025 · 2 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@cmaloney
Copy link
Contributor

cmaloney commented Mar 3, 2025 •
edited by bedevere-app bot
Loading

Feature or enhancement

Proposal:

This may indicate accidental data loss.

Ways to make sure all data is written:

  1. Use the file-like object as a “With Statement Context Manager”.
  2. Ensure .close() is always called which flushes data before closing.
  3. If the underlying stream need to be kept open, use .detach()

Since 3.12 flushing has been necessary in GzipFile (see gh-105808 which was a release blocker), this makes that more visible. Users have been encountering as they upgrade to 3.12 (ex. gh-129726).

There are a number of cases of unclosed file-like objects being deleted in CPython libraries and the test suite. This issue includes resolving those cases where the new ResourceWarning is emitted.

cc: @vstinner

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

#129726 (comment)

Linked PRs
@cmaloney cmaloney added the type-feature A feature request or enhancement label Mar 3, 2025
@encukou encukou added the stdlib Python modules in the Lib dir label Mar 4, 2025
cmaloney added a commit to cmaloney/cpython that referenced this issue Mar 6, 2025
@cmaloney
pythongh-130806: emit ResourceWarning if GzipFile unclosed
8bf1895 
This may indicate accidental data loss.

Ways to make sure all data is written:
1. Use the [file-like object](https://docs.python.org/3/glossary.html#term-file-object) as a [“With Statement Context Manager”](https://docs.python.org/3/reference/datamodel.html#context-managers).
   - All objects which [inherit](https://docs.python.org/3/tutorial/classes.html#inheritance) from [IOBase](https://docs.python.org/3/library/io.html#io.IOBase) support this.
   - [`BufferedIOBase`](https://docs.python.org/3/library/io.html#io.BufferedIOBase), [`BufferedWriter`](https://docs.python.org/3/library/io.html#io.BufferedWriter), and [`GzipFile`](https://docs.python.org/3/library/gzip.html#gzip.GzipFile) all support this.
   - Ensures `.close()` is called in both exception and regular cases.
1. Ensure [`.close()`](https://docs.python.org/3/library/io.html#io.IOBase.close) is always called which flushes data before closing.
1. If the underlying stream need to be kept open, use [`.detach()`](https://docs.python.org/3/library/io.html#io.BufferedIOBase.detach)

Since 3.12 flushing has been necessary in GzipFile (see pythongh-105808 which was a release blocker), this makes that more visible. Users have been encountering as they upgrade to 3.12 (ex. pythongh-129726).

There are a number of cases of unclosed file-like objects being deleted in CPython libraries and the test suite. This issue includes resolving those cases where the new ResourceWarning is emitted.
@bedevere-app bedevere-app bot mentioned this issue Mar 6, 2025
Merged
@cmaloney
Copy link
Contributor Author

Working on getting BufferedWriter warning to work, having a hard time as it involves adding a tp_finalize, and that interacts with the existing tp_dealloc which calls _dealloc_warn on itself and the underlying raw (often FileIO), which is how the FileIO close warning is emitted currently. I got it working well for _pyio but not sure how to get everything to fit for _io.Buffered{Writer,Random}. What I currently have. At the moment struggling to figure out whats the right tweaks, particularly for tests which seem to have often evolved separately from the original need, and this changes behavior (ex. _pyio tests using _io by accident, checks for no warning and unraisable exceptions, etc). Wondering if with tp_finalize as it currently works could replace the existing _dealloc_warn / FileIO system with a simpler one...

While working on that, found the docs around tp_finalize pointed to the deprecated C APIs PyErr_Fetch + PyErr_Restore, made an issue + PR to update to PyErr_GetRaisedException and PyErr_SetRaisedException: gh-131117

cc: @vstinner

vstinner added a commit that referenced this issue Mar 13, 2025
@cmaloney @vstinner
gh-130806: Emit ResourceWarning if GzipFile unclosed (#130905)
93089c0 
This may indicate accidental data loss.

Ways to make sure all data is written:
1. Use the [file-like object](https://docs.python.org/3/glossary.html#term-file-object) as a [“With Statement Context Manager”](https://docs.python.org/3/reference/datamodel.html#context-managers).
   - All objects which [inherit](https://docs.python.org/3/tutorial/classes.html#inheritance) from [IOBase](https://docs.python.org/3/library/io.html#io.IOBase) support this.
   - [`BufferedIOBase`](https://docs.python.org/3/library/io.html#io.BufferedIOBase), [`BufferedWriter`](https://docs.python.org/3/library/io.html#io.BufferedWriter), and [`GzipFile`](https://docs.python.org/3/library/gzip.html#gzip.GzipFile) all support this.
   - Ensures `.close()` is called in both exception and regular cases.

2. Ensure [`.close()`](https://docs.python.org/3/library/io.html#io.IOBase.close) is always called which flushes data before closing.

3. If the underlying stream need to be kept open, use [`.detach()`](https://docs.python.org/3/library/io.html#io.BufferedIOBase.detach)

Since 3.12 flushing has been necessary in GzipFile (see gh-105808 which was a release blocker), this makes that more visible. Users have been encountering as they upgrade to 3.12 (ex. gh-129726).

There are a number of cases of unclosed file-like objects being deleted in CPython libraries and the test suite. This issue includes resolving those cases where the new ResourceWarning is emitted.

Co-authored-by: Victor Stinner <[email protected]>
plashchynski pushed a commit to plashchynski/cpython that referenced this issue Mar 17, 2025
@cmaloney @vstinner @plashchynski
pythongh-130806: Emit ResourceWarning if GzipFile unclosed (python#13…
0158b99 
…0905)

This may indicate accidental data loss.

Ways to make sure all data is written:
1. Use the [file-like object](https://docs.python.org/3/glossary.html#term-file-object) as a [“With Statement Context Manager”](https://docs.python.org/3/reference/datamodel.html#context-managers).
   - All objects which [inherit](https://docs.python.org/3/tutorial/classes.html#inheritance) from [IOBase](https://docs.python.org/3/library/io.html#io.IOBase) support this.
   - [`BufferedIOBase`](https://docs.python.org/3/library/io.html#io.BufferedIOBase), [`BufferedWriter`](https://docs.python.org/3/library/io.html#io.BufferedWriter), and [`GzipFile`](https://docs.python.org/3/library/gzip.html#gzip.GzipFile) all support this.
   - Ensures `.close()` is called in both exception and regular cases.

2. Ensure [`.close()`](https://docs.python.org/3/library/io.html#io.IOBase.close) is always called which flushes data before closing.

3. If the underlying stream need to be kept open, use [`.detach()`](https://docs.python.org/3/library/io.html#io.BufferedIOBase.detach)

Since 3.12 flushing has been necessary in GzipFile (see pythongh-105808 which was a release blocker), this makes that more visible. Users have been encountering as they upgrade to 3.12 (ex. pythongh-129726).

There are a number of cases of unclosed file-like objects being deleted in CPython libraries and the test suite. This issue includes resolving those cases where the new ResourceWarning is emitted.

Co-authored-by: Victor Stinner <[email protected]>
@cmaloney cmaloney mentioned this issue Mar 19, 2025
Closed
@cmaloney cmaloney changed the title Emit ResourceWarning when GzipFile or BufferedWriter are deleted with unwritten data Emit ResourceWarning when GzipFile is deleted with unwritten data Mar 20, 2025
@cmaloney
Copy link
Contributor Author

Reducing the issue to just GzipFile, after a couple attempts at getting BufferedWriter working, have yet to find a reliable way. Is really neat how _dealloc_warn works with the tp_finalize in IOBase.

seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
@cmaloney @vstinner @seehwan80
pythongh-130806: Emit ResourceWarning if GzipFile unclosed (python#13…
1e419d2 
…0905)

This may indicate accidental data loss.

Ways to make sure all data is written:
1. Use the [file-like object](https://docs.python.org/3/glossary.html#term-file-object) as a [“With Statement Context Manager”](https://docs.python.org/3/reference/datamodel.html#context-managers).
   - All objects which [inherit](https://docs.python.org/3/tutorial/classes.html#inheritance) from [IOBase](https://docs.python.org/3/library/io.html#io.IOBase) support this.
   - [`BufferedIOBase`](https://docs.python.org/3/library/io.html#io.BufferedIOBase), [`BufferedWriter`](https://docs.python.org/3/library/io.html#io.BufferedWriter), and [`GzipFile`](https://docs.python.org/3/library/gzip.html#gzip.GzipFile) all support this.
   - Ensures `.close()` is called in both exception and regular cases.

2. Ensure [`.close()`](https://docs.python.org/3/library/io.html#io.IOBase.close) is always called which flushes data before closing.

3. If the underlying stream need to be kept open, use [`.detach()`](https://docs.python.org/3/library/io.html#io.BufferedIOBase.detach)

Since 3.12 flushing has been necessary in GzipFile (see pythongh-105808 which was a release blocker), this makes that more visible. Users have been encountering as they upgrade to 3.12 (ex. pythongh-129726).

There are a number of cases of unclosed file-like objects being deleted in CPython libraries and the test suite. This issue includes resolving those cases where the new ResourceWarning is emitted.

Co-authored-by: Victor Stinner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@encukou @cmaloney