classification
Title: Some StringIO and BytesIO methods can succeed after close
Type: behavior Stage: resolved
Components: IO, Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, eng793, hynek, pitrou, python-dev, stutzbach
Priority: normal Keywords: patch

Created on 2012-09-01 19:18 by pitrou, last changed 2012-09-05 18:21 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
stringio.c.patch eng793, 2012-09-02 10:08 patch for stringsio.c review
memio.patch eng793, 2012-09-02 14:01 patch review
memoryio.patch eng793, 2012-09-02 19:17 patch review
memoryio.patch eng793, 2012-09-04 18:04 patch review
Messages (11)
msg169665 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-09-01 19:18
>>> f = io.StringIO()
>>> f.close()
>>> f.readable()
True
>>> f.read()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: I/O operation on closed file.

>>> f = io.BytesIO()
>>> f.close()
>>> f.readable()
True
>>> f.read()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: I/O operation on closed file
msg169690 - (view) Author: Alessandro Moura (eng793) * Date: 2012-09-02 10:08
This also happens for the writable() and seekable() methods. The problem is that those methods do not check whether the buffers have been closed in stringio.c. This is fixed in the attached patch for StringIO. BytesIO should be the same, but bytesio.c is structured differently, and I still have to understand the code. I will try to do this, and then add tests for this issue - which should go in one of the mixins of test_memoryio.py, I presume.
msg169698 - (view) Author: Alessandro Moura (eng793) * Date: 2012-09-02 14:01
Here is the patch fixing the issue in both StringIO and BytesIO. In both cases, the problem is that in their respective c files, these methods always returned true, without testing whether the file was closed. Is this a recent rewrite? I am surprised this did not bite someone earlier.

I also added a testcase to test_memoryio.py covering this issue; and I added the docstring for these methods, which were missing.

This patch compiles and passes the entire test suite in my system.
msg169699 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-09-02 14:08
Ah, there's a misunderstanding. The methods should raise ValueError when the object has been closed.
See issue15840 for reference.
msg169719 - (view) Author: Alessandro Moura (eng793) * Date: 2012-09-02 19:17
Sorry, I should have seen the related issue (or just read the docs :)).

Here is the patch fixing these issues, implementing the behaviour stated in the docs (raising ValueError after close). The tests revealed that the _pyio module also needed fixing, and this is done in the patch as well.
msg169835 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-09-04 15:22
A couple of nits about the docstrings:

+PyDoc_STRVAR(stringio_seekable_doc,
+"seekable() -> Bool. Returns True if IO object can be seeked.");

First, "bool" should be lowercase. Also, in seekable(), "IO object" should be "the IO object".
Otherwise, looks fine to me.
msg169837 - (view) Author: Alessandro Moura (eng793) * Date: 2012-09-04 18:04
Thanks. Here is the amended patch with your suggestions implemented.
msg169845 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-09-04 23:31
Alessandro, have you already signed a contributor agreement so that your patch can be integrated?
If not, you'll find the instructions at http://www.python.org/psf/contrib/
Thank you!
msg169864 - (view) Author: Alessandro Moura (eng793) * Date: 2012-09-05 10:26
I just emailed my contributor's agreement.
msg169884 - (view) Author: Roundup Robot (python-dev) Date: 2012-09-05 18:20
New changeset 524931e5aebf by Antoine Pitrou in branch '3.2':
Issue #15841: The readable(), writable() and seekable() methods of BytesIO
http://hg.python.org/cpython/rev/524931e5aebf

New changeset fcf097cb5f6b by Antoine Pitrou in branch 'default':
Issue #15841: The readable(), writable() and seekable() methods of BytesIO
http://hg.python.org/cpython/rev/fcf097cb5f6b

New changeset d0ab34d4c733 by Antoine Pitrou in branch '2.7':
Issue #15841: The readable(), writable() and seekable() methods of io.BytesIO
http://hg.python.org/cpython/rev/d0ab34d4c733
msg169885 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-09-05 18:21
I have now committed the patch. Thank you again!
History
Date User Action Args
2012-09-05 18:21:33pitrousetstatus: open -> closed
resolution: fixed
messages: + msg169885

stage: resolved
2012-09-05 18:20:50python-devsetnosy: + python-dev
messages: + msg169884
2012-09-05 10:26:35eng793setmessages: + msg169864
2012-09-04 23:31:47pitrousetmessages: + msg169845
2012-09-04 18:04:31eng793setfiles: + memoryio.patch

messages: + msg169837
2012-09-04 15:22:01pitrousetmessages: + msg169835
2012-09-02 19:17:08eng793setfiles: + memoryio.patch

messages: + msg169719
2012-09-02 14:08:32pitrousetmessages: + msg169699
2012-09-02 14:01:19eng793setfiles: + memio.patch

messages: + msg169698
2012-09-02 10:08:19eng793setfiles: + stringio.c.patch

nosy: + eng793
messages: + msg169690

keywords: + patch
2012-09-01 19:19:03pitrousetnosy: + benjamin.peterson, stutzbach, hynek
2012-09-01 19:18:50pitroucreate