classification
Title: file descriptor not being closed with context manager on IOError when device is full
Type: behavior Stage: resolved
Components: Extension Modules, IO Versions: Python 3.4, Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, asvetlov, benjamin.peterson, christian.heimes, hynek, jcea, pitrou, python-dev, serhiy.storchaka, stutzbach, udoprog
Priority: normal Keywords: patch

Created on 2012-12-02 23:24 by udoprog, last changed 2012-12-20 19:11 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
bufferedio_finally_close.patch serhiy.storchaka, 2012-12-03 12:28 review
bufferedio_finally_close_2.patch serhiy.storchaka, 2012-12-20 17:37 review
bufferedio_finally_close-3.2_2.patch serhiy.storchaka, 2012-12-20 18:14 Patch for 3.2 review
bufferedio_finally_close-2.7_2.patch serhiy.storchaka, 2012-12-20 18:18 Patch for 2.7 review
Messages (17)
msg176816 - (view) Author: John-John Tedro (udoprog) Date: 2012-12-02 23:24
In 3.2.2 and 3.2.3 on linux64, when running the following code. 

    try:
        print("Writing to /dev/full")

        with open("/dev/full", "w") as f:
             f.write("Write to full device")
    except:
        print("Catch, file closed?")

Using

    strace -e close ~/usr/python3.2.3/bin/python3 test.py
    ...
    Writing to /dev/full
    Catch, file closed?
    close(3)                                = 0

The file descriptor being used (3) to attempt writing to /dev/full is not closed until the process exits.

I expected this to be closed when leaving the context manager.
msg176818 - (view) Author: John-John Tedro (udoprog) Date: 2012-12-02 23:31
Originally discovered on http://stackoverflow.com/questions/13649330/what-happens-to-file-descriptors-in-python-3-when-close-fails
msg176830 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-03 09:22
>>> f = open("/dev/full", "wb", buffering=0)
>>> f.write(b"Write to full device")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 28] No space left on device
>>> f.close()
>>> f.closed
True
>>> f = open("/dev/full", "wb")
>>> f.write(b"Write to full device")
20
>>> f.close()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 28] No space left on device
>>> f.closed
False


Python 2 has the same behavior using io.open.
msg176831 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-03 09:24
The bug only in C implementation.

>>> import _pyio
>>> f = _pyio.open("/dev/full", "wb")
>>> f.write(b"Write to full device")
20
>>> f.close()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/serhiy/py/cpython/Lib/_pyio.py", line 732, in close
    self.flush()
  File "/home/serhiy/py/cpython/Lib/_pyio.py", line 1121, in flush
    self._flush_unlocked()
  File "/home/serhiy/py/cpython/Lib/_pyio.py", line 1128, in _flush_unlocked
    n = self.raw.write(self._write_buf)
OSError: [Errno 28] No space left on device
>>> f.closed
True
msg176839 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-03 12:25
Here is a patch which calls close() on underlying stream even if flush() raises an exception.

I am not sure that I correctly set a context exception. There is no other examples in the code.
msg177832 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-20 16:15
There are a lot of people in the nosy list already. Does anyone have enough experience with exception machinery to review the patch (especially C part)?
msg177838 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-20 17:37
Patch updated with Benjamin's nit.
msg177840 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-20 17:55
New changeset b0602a1eb3f6 by Benjamin Peterson in branch '3.3':
call close on the underlying stream even if flush raises (closes #16597)
http://hg.python.org/cpython/rev/b0602a1eb3f6

New changeset 142012e47c3b by Benjamin Peterson in branch 'default':
merge 3.3 (#16597)
http://hg.python.org/cpython/rev/142012e47c3b
msg177842 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-20 18:14
Here is a patch for 3.2.
msg177843 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-20 18:18
Here is a patch for 2.7.
msg177846 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2012-12-20 18:23
I think we can leave 3.2 alone.
msg177847 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-20 18:24
New changeset b6ff6ac1f049 by Benjamin Peterson in branch '2.7':
call close on the underlying stream even if flush raises (#16597)
http://hg.python.org/cpython/rev/b6ff6ac1f049
msg177848 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-20 18:26
What is the peculiarity of 3.2?
msg177851 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-20 18:52
About Misc/NEWS. Actually fixed close() methods of: Python implementation of BaseIO (C implementation already do right things), C implemettation of Buffered(Reader|Writer|Random) (Python implementation already do right things) and both implementations of TextIOWrapper.
msg177852 - (view) Author: Roundup Robot (python-dev) Date: 2012-12-20 18:56
New changeset 54372f38932e by Benjamin Peterson in branch '3.3':
improve message (#16597)
http://hg.python.org/cpython/rev/54372f38932e

New changeset faaac6ceffff by Benjamin Peterson in branch 'default':
merge 3.3 (#16597)
http://hg.python.org/cpython/rev/faaac6ceffff

New changeset a057d9985fb8 by Benjamin Peterson in branch '2.7':
add news note (#16597)
http://hg.python.org/cpython/rev/a057d9985fb8
msg177853 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-20 19:10
Thank you for all commits, Benjamin.

What is the peculiarity of 3.2?
msg177854 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2012-12-20 19:11
2012/12/20 Serhiy Storchaka <report@bugs.python.org>:
>
> Serhiy Storchaka added the comment:
>
> Thank you for all commits, Benjamin.
>
> What is the peculiarity of 3.2?

Applying bugfixes is optional. It should move to security-fix mode soon.
History
Date User Action Args
2015-09-21 15:03:28zach.warelinkissue25202 superseder
2012-12-20 19:11:20benjamin.petersonsetmessages: + msg177854
2012-12-20 19:10:31serhiy.storchakasetmessages: + msg177853
2012-12-20 18:56:07python-devsetmessages: + msg177852
2012-12-20 18:52:06serhiy.storchakasetmessages: + msg177851
2012-12-20 18:26:55serhiy.storchakasetmessages: + msg177848
2012-12-20 18:24:22python-devsetmessages: + msg177847
2012-12-20 18:23:57benjamin.petersonsetmessages: + msg177846
2012-12-20 18:18:53serhiy.storchakasetfiles: + bufferedio_finally_close-2.7_2.patch

messages: + msg177843
2012-12-20 18:14:47serhiy.storchakasetfiles: + bufferedio_finally_close-3.2_2.patch

messages: + msg177842
2012-12-20 17:55:28python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg177840

resolution: fixed
stage: patch review -> resolved
2012-12-20 17:37:53serhiy.storchakasetfiles: + bufferedio_finally_close_2.patch

messages: + msg177838
2012-12-20 16:15:30serhiy.storchakasetmessages: + msg177832
2012-12-20 14:13:06christian.heimessetnosy: + christian.heimes
2012-12-16 13:41:20pitrousetnosy: + hynek
2012-12-15 18:01:09asvetlovsetnosy: + asvetlov
2012-12-05 19:22:41jceasetnosy: + jcea
2012-12-03 12:28:51serhiy.storchakasetfiles: + bufferedio_finally_close.patch
2012-12-03 12:27:52serhiy.storchakasetfiles: - bufferedio_finally_close.patch
2012-12-03 12:26:00serhiy.storchakasetfiles: + bufferedio_finally_close.patch

nosy: + pitrou, benjamin.peterson, stutzbach
messages: + msg176839

keywords: + patch
stage: patch review
2012-12-03 09:24:50serhiy.storchakasetmessages: + msg176831
components: + Extension Modules
2012-12-03 09:22:21serhiy.storchakasetnosy: + serhiy.storchaka

messages: + msg176830
versions: + Python 2.7, Python 3.3, Python 3.4
2012-12-03 07:54:13Arfreversetnosy: + Arfrever
2012-12-02 23:31:08udoprogsetmessages: + msg176818
2012-12-02 23:27:35udoprogsettitle: close not being called with context manager on IOError when device is full. -> file descriptor not being closed with context manager on IOError when device is full
2012-12-02 23:24:53udoprogcreate