From e05565ec5af6b326e9d1f17c3f79bc24bf1ee157 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Sat, 20 Aug 2011 14:39:23 +0200 Subject: Issue #12213: Fix a buffering bug with interleaved reads and writes that could appear on BufferedRandom streams. --- Lib/test/test_io.py | 44 +++++++++++++++++- Misc/NEWS | 3 ++ Modules/_io/bufferedio.c | 117 +++++++++++++++++++++++------------------------ 3 files changed, 103 insertions(+), 61 deletions(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index d32580c..72c9a2d 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -1395,15 +1395,18 @@ class BufferedRandomTest(BufferedReaderTest, BufferedWriterTest): 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 @@ class BufferedRandomTest(BufferedReaderTest, BufferedWriterTest): 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 index f8c135f..da0e67f 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -19,6 +19,9 @@ Core and Builtins 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 index 26ebde0..d6f0c9c 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -752,25 +752,38 @@ _trap_eintr(void) */ static PyObject * -buffered_flush(buffered *self, PyObject *args) +buffered_flush_and_rewind_unlocked(buffered *self) { PyObject *res; - CHECK_INITIALIZED(self) - CHECK_CLOSED(self, "flush of closed file") - - if (!ENTER_BUFFERED(self)) - return NULL; res = _bufferedwriter_flush_unlocked(self, 0); - if (res != NULL && self->readable) { + 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); - if (n == -1) - Py_CLEAR(res); _bufferedreader_reset_buf(self); + if (n == -1) + return NULL; } + Py_RETURN_NONE; +} + +static PyObject * +buffered_flush(buffered *self, PyObject *args) +{ + PyObject *res; + + CHECK_INITIALIZED(self) + CHECK_CLOSED(self, "flush of closed file") + + if (!ENTER_BUFFERED(self)) + return NULL; + res = buffered_flush_and_rewind_unlocked(self); LEAVE_BUFFERED(self) return res; @@ -791,7 +804,7 @@ buffered_peek(buffered *self, PyObject *args) 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 @@ buffered_read(buffered *self, PyObject *args) 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 @@ buffered_read1(buffered *self, PyObject *args) 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 @@ buffered_read1(buffered *self, PyObject *args) 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 @@ end: 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 @@ _buffered_readline(buffered *self, Py_ssize_t limit) 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 @@ _buffered_readline(buffered *self, Py_ssize_t limit) } 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 @@ buffered_truncate(buffered *self, PyObject *args) 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 @@ _bufferedreader_read_all(buffered *self) 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 @@ _bufferedreader_read_generic(buffered *self, Py_ssize_t n) 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) { -- cgit v0.12