classification
Title: BufferedRandom: issues with interlaced read-write
Type: behavior Stage: resolved
Components: IO Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, haypo, jcea, pitrou, python-dev, santoso.wijaya
Priority: critical Keywords: patch

Created on 2011-05-30 02:52 by haypo, last changed 2011-09-04 06:41 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
io_interlaced_read_write.patch haypo, 2011-05-30 11:57 review
bufrandom.patch pitrou, 2011-08-19 18:33
Messages (15)
msg137238 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-05-30 02:52
The following code displays "Xbc" using io, and "bc" using _pyio (or an unbuffered file, e.g. io.FileIO):
-------------
import _pyio, io

with io.BytesIO(b'abc') as raw:
    #with _pyio.BufferedRandom(raw) as f:
    with io.BufferedRandom(raw) as f:
        f.write(b"X")
        print("pos?", f.tell(), raw.tell())
        print(f.read())
-------------

I expect .write() to change the file position, and so "bc" must be the correct result, not "Wbc".

_pyio.BufferedRandom overrides its write method, whereas io.BufferedRandom doesn't.

I already noticed the implement difference of BufferedRandom.write(), but I don't remember if I reported it or not!?
msg137239 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-05-30 02:54
> I already noticed the implement difference of BufferedRandom.write(),
> but I don't remember if I reported it or not!?

Yes, I already reported the difference:
http://bugs.python.org/issue12062#msg135876
msg137259 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-05-30 11:57
io_interlaced_read_write.patch:
 - add interlaced read/write tests for BufferedRandom and BufferedRWPair
 - _pyio: move "undo readahead" code into BufferedReader.flush()
 - io: BufferedRandom.flush() doesn't undo readahead if the write buffer is empty, so it's possible to call it in read methods without flusing the readahead buffer when it's not needed
 - read(), read1(), readinto(), peek() calls writer.flush()
 - write() calls reader.flush() (undo readahead)

TODO:
 - tests BufferedRWPair with read-only + write-only files because I'm not sure if my tests using readable and writeable methods
 - _pyio: undo the readahead in BufferedReader.flush() is no perfect, because BufferedReader is supposed to be read-only. I choosed that to factorize the code between BufferedRandom and BufferedRWPair
 - what happens if a write occurs during _pyio.BufferedReader.flush()? "if self._read_buf: <write occurs> with self._read_lock: ...". We may protect the read of self._read_buf with the read lock.

My patch tries to fix interlaced read-write by always calling flush(), but I am not sure that it doesn't change read-only and write-only cases. There are maybe some unnecessary call to flush().
msg137261 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-05-30 12:38
See also issue #12215: TextIOWrapper has also issues on interlaced read-write.
msg137690 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-06-05 11:25
> My patch tries to fix interlaced read-write by always calling flush(),

Why do you need to call flush()? Can't you read from the buffer?
msg138736 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-06-20 16:01
>> My patch tries to fix interlaced read-write by always calling flush(),
>
> Why do you need to call flush()? Can't you read from the buffer?

Hum, my patch does not always call flush of the reader and the writer. On read, it flushs the writer. On write, it "flushes" the reader (undo readahead).

It is maybe possible to do better (do something faster), but there should be some tricky cases with seek().
msg142479 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-19 17:10
I think your expectations for BufferedRWPair are wrong. You should not use BufferedRWPair with the same underlying stream (that's the whole point of BufferedRWPair).
msg142487 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-19 18:33
Here is a patch.
msg142528 - (view) Author: Roundup Robot (python-dev) Date: 2011-08-20 12:54
New changeset 5330af45f777 by Antoine Pitrou in branch '3.2':
Issue #12213: Fix a buffering bug with interleaved reads and writes that
http://hg.python.org/cpython/rev/5330af45f777

New changeset d7f6391954cf by Antoine Pitrou in branch 'default':
Issue #12213: Fix a buffering bug with interleaved reads and writes that
http://hg.python.org/cpython/rev/d7f6391954cf
msg142529 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-20 12:55
This should be fixed for BufferedRandom. As I said, I don't think BufferedRWPair is buggy.
msg142530 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-08-20 13:16
> You should not use BufferedRWPair with the same underlying stream (that's the whole point of BufferedRWPair).

It might be documented. Something like "Warning: don't use the same 
stream as reader and writer, or the BufferedRWPair becomes inconsistent 
on interlaced read-write" ?
msg142532 - (view) Author: Roundup Robot (python-dev) Date: 2011-08-20 13:43
New changeset cf2010e9f941 by Antoine Pitrou in branch '2.7':
Issue #12213: Fix a buffering bug with interleaved reads and writes that
http://hg.python.org/cpython/rev/cf2010e9f941
msg142545 - (view) Author: Roundup Robot (python-dev) Date: 2011-08-20 17:56
New changeset 524896c305ce by Antoine Pitrou in branch '3.2':
Issue #12213: make it clear that BufferedRWPair shouldn't be called with the
http://hg.python.org/cpython/rev/524896c305ce

New changeset a423bd492d6c by Antoine Pitrou in branch 'default':
Issue #12213: make it clear that BufferedRWPair shouldn't be called with the
http://hg.python.org/cpython/rev/a423bd492d6c

New changeset dd4f29e39756 by Antoine Pitrou in branch '2.7':
Issue #12213: make it clear that BufferedRWPair shouldn't be called with the
http://hg.python.org/cpython/rev/dd4f29e39756
msg142546 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-20 17:56
I think it can be closed now.
msg143476 - (view) Author: Roundup Robot (python-dev) Date: 2011-09-04 06:41
New changeset 8de8945ebfc3 by Antoine Pitrou in branch '3.2':
Issue #12213: make it clear that BufferedRWPair shouldn't be called with the
http://hg.python.org/cpython/rev/8de8945ebfc3
History
Date User Action Args
2011-09-04 06:41:30python-devsetmessages: + msg143476
2011-08-20 17:56:47pitrousetstatus: open -> closed

messages: + msg142546
2011-08-20 17:56:18python-devsetmessages: + msg142545
2011-08-20 15:15:22pitrousettitle: BufferedRandom, BufferedRWPair: issues with interlaced read-write -> BufferedRandom: issues with interlaced read-write
2011-08-20 15:12:40Arfreversettitle: BufferedRandom, BufferedRWPair: issues with interlaced read-write -> BufferedRandom, BufferedRWPair: issues with interlaced read-write
2011-08-20 13:43:18python-devsetmessages: + msg142532
2011-08-20 13:16:21hayposetstatus: pending -> open

messages: + msg142530
title: BufferedRandom: issues with interlaced read-write -> BufferedRandom, BufferedRWPair: issues with interlaced read-write
2011-08-20 12:55:08pitrousetstatus: open -> pending
title: BufferedRandom, BufferedRWPair: issues with interlaced read-write -> BufferedRandom: issues with interlaced read-write
messages: + msg142529

resolution: fixed
stage: patch review -> resolved
2011-08-20 12:54:24python-devsetnosy: + python-dev
messages: + msg142528
2011-08-19 18:33:25pitrousetfiles: + bufrandom.patch

messages: + msg142487
stage: patch review
2011-08-19 17:10:22pitrousetmessages: + msg142479
2011-07-01 18:54:03pitrousetpriority: normal -> critical
type: behavior
2011-07-01 16:50:53Arfreversetnosy: + Arfrever
2011-06-20 16:47:43santoso.wijayasetnosy: + santoso.wijaya
2011-06-20 16:01:05hayposetmessages: + msg138736
2011-06-05 11:25:41pitrousetmessages: + msg137690
2011-05-30 13:08:27hayposetversions: + Python 2.7, Python 3.2
2011-05-30 12:38:36hayposetmessages: + msg137261
2011-05-30 11:57:30hayposetfiles: + io_interlaced_read_write.patch
keywords: + patch
messages: + msg137259
2011-05-30 11:30:46hayposettitle: BufferedRandom: write(); read() gives different result using io and _pyio -> BufferedRandom, BufferedRWPair: issues with interlaced read-write
2011-05-30 03:20:08jceasetnosy: + jcea
2011-05-30 02:54:51hayposetmessages: + msg137239
2011-05-30 02:52:58hayposetcomponents: + IO
2011-05-30 02:52:39haypocreate