[Python-checkins] cpython (3.2): Issue #12213: Fix a buffering bug with interleaved reads and writes that

Sat Aug 20 14:54:22 CEST 2011

http://hg.python.org/cpython/rev/5330af45f777
changeset: 71982:5330af45f777
branch: 3.2
parent: 71980:867ce75b885c
user: Antoine Pitrou <solipsis at pitrou.net>
date: Sat Aug 20 14:39:23 2011 +0200
summary:
 Issue #12213: Fix a buffering bug with interleaved reads and writes that
could appear on BufferedRandom streams.
files:
 Lib/test/test_io.py | 44 +++++++++-
 Misc/NEWS | 3 +
 Modules/_io/bufferedio.c | 119 +++++++++++++-------------
 3 files changed, 104 insertions(+), 62 deletions(-)

diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -1395,15 +1395,18 @@
 rw.seek(0, 0)
 self.assertEqual(b"asdf", rw.read(4))
 
- rw.write(b"asdf")
+ rw.write(b"123f")
 rw.seek(0, 0)
- self.assertEqual(b"asdfasdfl", rw.read())
+ self.assertEqual(b"asdf123fl", rw.read())
 self.assertEqual(9, rw.tell())
 rw.seek(-4, 2)
 self.assertEqual(5, rw.tell())
 rw.seek(2, 1)
 self.assertEqual(7, rw.tell())
 self.assertEqual(b"fl", rw.read(11))
+ rw.flush()
+ self.assertEqual(b"asdf123fl", raw.getvalue())
+
 self.assertRaises(TypeError, rw.seek, 0.0)
 
 def check_flush_and_read(self, read_func):
@@ -1548,6 +1551,43 @@
 BufferedReaderTest.test_misbehaved_io(self)
 BufferedWriterTest.test_misbehaved_io(self)
 
+ def test_interleaved_read_write(self):
+ # Test for issue #12213
+ with self.BytesIO(b'abcdefgh') as raw:
+ with self.tp(raw, 100) as f:
+ f.write(b"1")
+ self.assertEqual(f.read(1), b'b')
+ f.write(b'2')
+ self.assertEqual(f.read1(1), b'd')
+ f.write(b'3')
+ buf = bytearray(1)
+ f.readinto(buf)
+ self.assertEqual(buf, b'f')
+ f.write(b'4')
+ self.assertEqual(f.peek(1), b'h')
+ f.flush()
+ self.assertEqual(raw.getvalue(), b'1b2d3f4h')
+
+ with self.BytesIO(b'abc') as raw:
+ with self.tp(raw, 100) as f:
+ self.assertEqual(f.read(1), b'a')
+ f.write(b"2")
+ self.assertEqual(f.read(1), b'c')
+ f.flush()
+ self.assertEqual(raw.getvalue(), b'a2c')
+
+ def test_interleaved_readline_write(self):
+ with self.BytesIO(b'ab\ncdef\ng\n') as raw:
+ with self.tp(raw) as f:
+ f.write(b'1')
+ self.assertEqual(f.readline(), b'b\n')
+ f.write(b'2')
+ self.assertEqual(f.readline(), b'def\n')
+ f.write(b'3')
+ self.assertEqual(f.readline(), b'\n')
+ f.flush()
+ self.assertEqual(raw.getvalue(), b'1b\n2def\n3\n')
+
 # You can't construct a BufferedRandom over a non-seekable stream.
 test_unseekable = None
 
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -19,6 +19,9 @@
 Library
 -------
 
+- Issue #12213: Fix a buffering bug with interleaved reads and writes that
+ could appear on BufferedRandom streams.
+
 - Issue #12326: sys.platform is now always 'linux2' on Linux, even if Python
 is compiled on Linux 3.
 
diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c
--- a/Modules/_io/bufferedio.c
+++ b/Modules/_io/bufferedio.c
@@ -752,6 +752,28 @@
 */
 
 static PyObject *
+buffered_flush_and_rewind_unlocked(buffered *self)
+{
+ PyObject *res;
+
+ res = _bufferedwriter_flush_unlocked(self, 0);
+ if (res == NULL)
+ return NULL;
+ Py_DECREF(res);
+
+ if (self->readable) {
+ /* Rewind the raw stream so that its position corresponds to
+ the current logical position. */
+ Py_off_t n;
+ n = _buffered_raw_seek(self, -RAW_OFFSET(self), 1);
+ _bufferedreader_reset_buf(self);
+ if (n == -1)
+ return NULL;
+ }
+ Py_RETURN_NONE;
+}
+
+static PyObject *
 buffered_flush(buffered *self, PyObject *args)
 {
 PyObject *res;
@@ -761,16 +783,7 @@
 
 if (!ENTER_BUFFERED(self))
 return NULL;
- res = _bufferedwriter_flush_unlocked(self, 0);
- if (res != NULL && self->readable) {
- /* Rewind the raw stream so that its position corresponds to
- the current logical position. */
- Py_off_t n;
- n = _buffered_raw_seek(self, -RAW_OFFSET(self), 1);
- if (n == -1)
- Py_CLEAR(res);
- _bufferedreader_reset_buf(self);
- }
+ res = buffered_flush_and_rewind_unlocked(self);
 LEAVE_BUFFERED(self)
 
 return res;
@@ -791,7 +804,7 @@
 return NULL;
 
 if (self->writable) {
- res = _bufferedwriter_flush_unlocked(self, 1);
+ res = buffered_flush_and_rewind_unlocked(self);
 if (res == NULL)
 goto end;
 Py_CLEAR(res);
@@ -826,19 +839,18 @@
 if (!ENTER_BUFFERED(self))
 return NULL;
 res = _bufferedreader_read_all(self);
- LEAVE_BUFFERED(self)
 }
 else {
 res = _bufferedreader_read_fast(self, n);
- if (res == Py_None) {
- Py_DECREF(res);
- if (!ENTER_BUFFERED(self))
- return NULL;
- res = _bufferedreader_read_generic(self, n);
- LEAVE_BUFFERED(self)
- }
+ if (res != Py_None)
+ return res;
+ Py_DECREF(res);
+ if (!ENTER_BUFFERED(self))
+ return NULL;
+ res = _bufferedreader_read_generic(self, n);
 }
 
+ LEAVE_BUFFERED(self)
 return res;
 }
 
@@ -864,13 +876,6 @@
 if (!ENTER_BUFFERED(self))
 return NULL;
 
- if (self->writable) {
- res = _bufferedwriter_flush_unlocked(self, 1);
- if (res == NULL)
- goto end;
- Py_CLEAR(res);
- }
-
 /* Return up to n bytes. If at least one byte is buffered, we
 only return buffered bytes. Otherwise, we do one raw read. */
 
@@ -890,6 +895,13 @@
 goto end;
 }
 
+ if (self->writable) {
+ res = buffered_flush_and_rewind_unlocked(self);
+ if (res == NULL)
+ goto end;
+ Py_DECREF(res);
+ }
+
 /* Fill the buffer from the raw stream, and copy it to the result. */
 _bufferedreader_reset_buf(self);
 r = _bufferedreader_fill_buffer(self);
@@ -912,24 +924,10 @@
 static PyObject *
 buffered_readinto(buffered *self, PyObject *args)
 {
- PyObject *res = NULL;
-
 CHECK_INITIALIZED(self)
 
- /* TODO: use raw.readinto() instead! */
- if (self->writable) {
- if (!ENTER_BUFFERED(self))
- return NULL;
- res = _bufferedwriter_flush_unlocked(self, 0);
- LEAVE_BUFFERED(self)
- if (res == NULL)
- goto end;
- Py_DECREF(res);
- }
- res = bufferediobase_readinto((PyObject *)self, args);
-
-end:
- return res;
+ /* TODO: use raw.readinto() (or a direct copy from our buffer) instead! */
+ return bufferediobase_readinto((PyObject *)self, args);
 }
 
 static PyObject *
@@ -967,12 +965,6 @@
 goto end_unlocked;
 
 /* Now we try to get some more from the raw stream */
- if (self->writable) {
- res = _bufferedwriter_flush_unlocked(self, 1);
- if (res == NULL)
- goto end;
- Py_CLEAR(res);
- }
 chunks = PyList_New(0);
 if (chunks == NULL)
 goto end;
@@ -986,9 +978,16 @@
 }
 Py_CLEAR(res);
 written += n;
+ self->pos += n;
 if (limit >= 0)
 limit -= n;
 }
+ if (self->writable) {
+ PyObject *r = buffered_flush_and_rewind_unlocked(self);
+ if (r == NULL)
+ goto end;
+ Py_DECREF(r);
+ }
 
 for (;;) {
 _bufferedreader_reset_buf(self);
@@ -1157,20 +1156,11 @@
 return NULL;
 
 if (self->writable) {
- res = _bufferedwriter_flush_unlocked(self, 0);
+ res = buffered_flush_and_rewind_unlocked(self);
 if (res == NULL)
 goto end;
 Py_CLEAR(res);
 }
- if (self->readable) {
- if (pos == Py_None) {
- /* Rewind the raw stream so that its position corresponds to
- the current logical position. */
- if (_buffered_raw_seek(self, -RAW_OFFSET(self), 1) == -1)
- goto end;
- }
- _bufferedreader_reset_buf(self);
- }
 res = PyObject_CallMethodObjArgs(self->raw, _PyIO_str_truncate, pos, NULL);
 if (res == NULL)
 goto end;
@@ -1367,17 +1357,18 @@
 Py_DECREF(chunks);
 return NULL;
 }
+ self->pos += current_size;
 }
- _bufferedreader_reset_buf(self);
 /* We're going past the buffer's bounds, flush it */
 if (self->writable) {
- res = _bufferedwriter_flush_unlocked(self, 1);
+ res = buffered_flush_and_rewind_unlocked(self);
 if (res == NULL) {
 Py_DECREF(chunks);
 return NULL;
 }
 Py_CLEAR(res);
 }
+ _bufferedreader_reset_buf(self);
 while (1) {
 if (data) {
 if (PyList_Append(chunks, data) < 0) {
@@ -1460,6 +1451,14 @@
 memcpy(out, self->buffer + self->pos, current_size);
 remaining -= current_size;
 written += current_size;
+ self->pos += current_size;
+ }
+ /* Flush the write buffer if necessary */
+ if (self->writable) {
+ PyObject *r = buffered_flush_and_rewind_unlocked(self);
+ if (r == NULL)
+ goto error;
+ Py_DECREF(r);
 }
 _bufferedreader_reset_buf(self);
 while (remaining > 0) {
-- 
Repository URL: http://hg.python.org/cpython
