Skip to content

bpo-37223: io.BufferedWrite.close() don't call flush() if already closed #13999

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
vstinner wants to merge 2 commits into from
Closed

bpo-37223: io.BufferedWrite.close() don't call flush() if already closed #13999

vstinner wants to merge 2 commits into from

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Jun 12, 2019 •
edited by bedevere-bot
Loading

The close() method of io buffered files no longer call flush()
if the file is already closed.

https://bugs.python.org/issue37223

@vstinner vstinner requested a review from pitrou June 12, 2019 02:39
@vstinner
Copy link
Member Author

vstinner commented Jun 12, 2019

This PR fix the following 2 errors when running test_io:

test_writer_close_error_on_close (test.test_io.CBufferedRWPairTest) ... Exception ignored in: <_io.BufferedWriter>
Traceback (most recent call last):
  File "/home/vstinner/prog/python/master/Lib/unittest/case.py", line 611, in _callTestMethod
    method()
ValueError: flush of closed file
ok

test_destructor (test.test_io.CBufferedRandomTest) ... Exception ignored in: <_io.BufferedRWPair object at 0x7fc9d1c230b0>
Traceback (most recent call last):
  File "/home/vstinner/prog/python/master/Lib/test/support/__init__.py", line 1627, in gc_collect
    gc.collect()
ValueError: flush of closed file
Exception ignored in: <_io.BufferedWriter>
Traceback (most recent call last):
  File "/home/vstinner/prog/python/master/Lib/test/support/__init__.py", line 1627, in gc_collect
    gc.collect()
ValueError: flush of closed file
ok

@vstinner
bpo-37223: io.BufferedWrite.close() don't call flush() if already closed
6e7316d 
io.BufferedWrite.close() no longer call its flush() method if close()
has already been closed.

* buffered_close() no longer call buffered_flush()
  if self->buffer is NULL
* _pyio.BufferedWriter.close() now sets _write_buf to None. Moreover,
  it doesn't call flush() anymore if _write_buf is None.
* Add test_flush_error_on_close() to test_io.
@vstinner
Copy link
Member Author

vstinner commented Jun 12, 2019

I rebased my PR, added an unit test and I also fixed the pure Python implementation.

@vstinner
Enhance _pyio.BufferedWriter
6af7aa3 
Add more unit tests
@vstinner vstinner changed the title bpo-37223: Fix _io buffered_closed() function bpo-37223: io.BufferedWrite.close() don't call flush() if already closed Jun 12, 2019
@vstinner
Copy link
Member Author

vstinner commented Jun 12, 2019

This PR changes the behavior of the io module for a corner case. I merged PR #14028 instead to only fix an error when running test_io. IMHO the current behavior makes sense and ensures that text, buffered and fileio states are consistent, especially for the closed attribute. I close this PR.

@vstinner vstinner closed this Jun 12, 2019
@vstinner vstinner deleted the buffered_close branch June 12, 2019 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitrou pitrou Awaiting requested review from pitrou

Assignees

No one assigned

Labels

awaiting core review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vstinner @the-knights-who-say-ni @bedevere-bot