diff options
| -rw-r--r-- | Lib/test/test_io.py | 38 | ||||
| -rw-r--r-- | Misc/NEWS | 4 | ||||
| -rw-r--r-- | Modules/_io/bufferedio.c | 71 |
3 files changed, 94 insertions, 19 deletions
diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index bcf435f..a17fcee 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -2653,12 +2653,50 @@ class SignalsTest(unittest.TestCase): def test_interrupted_write_text(self): self.check_interrupted_write("xy", b"xy", mode="w", encoding="ascii") + def check_reentrant_write(self, data, **fdopen_kwargs): + def on_alarm(*args): + # Will be called reentrantly from the same thread + wio.write(data) + 1/0 + signal.signal(signal.SIGALRM, on_alarm) + r, w = os.pipe() + wio = self.io.open(w, **fdopen_kwargs) + try: + signal.alarm(1) + # Either the reentrant call to wio.write() fails with RuntimeError, + # or the signal handler raises ZeroDivisionError. + with self.assertRaises((ZeroDivisionError, RuntimeError)) as cm: + while 1: + for i in range(100): + wio.write(data) + wio.flush() + # Make sure the buffer doesn't fill up and block further writes + os.read(r, len(data) * 100) + exc = cm.exception + if isinstance(exc, RuntimeError): + self.assertTrue(str(exc).startswith("reentrant call"), str(exc)) + finally: + wio.close() + os.close(r) + + def test_reentrant_write_buffered(self): + self.check_reentrant_write(b"xy", mode="wb") + + def test_reentrant_write_text(self): + self.check_reentrant_write("xy", mode="w", encoding="ascii") + + class CSignalsTest(SignalsTest): io = io class PySignalsTest(SignalsTest): io = pyio + # Handling reentrancy issues would slow down _pyio even more, so the + # tests are disabled. + test_reentrant_write_buffered = None + test_reentrant_write_text = None + def test_main(): tests = (CIOTest, PyIOTest, @@ -35,6 +35,10 @@ Core and Builtins Library ------- +- Issue #10478: Reentrant calls inside buffered IO objects (for example by + way of a signal handler) now raise a RuntimeError instead of freezing the + current process. + - logging: Added getLogRecordFactory/setLogRecordFactory with docs and tests. - Issue #10549: Fix pydoc traceback when text-documenting certain classes. diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 7661a00..f7e34a8 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -225,6 +225,7 @@ typedef struct { #ifdef WITH_THREAD PyThread_type_lock lock; + volatile long owner; #endif Py_ssize_t buffer_size; @@ -260,17 +261,34 @@ typedef struct { /* These macros protect the buffered object against concurrent operations. */ #ifdef WITH_THREAD -#define ENTER_BUFFERED(self) \ - if (!PyThread_acquire_lock(self->lock, 0)) { \ - Py_BEGIN_ALLOW_THREADS \ - PyThread_acquire_lock(self->lock, 1); \ - Py_END_ALLOW_THREADS \ + +static int +_enter_buffered_busy(buffered *self) +{ + if (self->owner == PyThread_get_thread_ident()) { + PyErr_Format(PyExc_RuntimeError, + "reentrant call inside %R", self); + return 0; } + Py_BEGIN_ALLOW_THREADS + PyThread_acquire_lock(self->lock, 1); + Py_END_ALLOW_THREADS + return 1; +} + +#define ENTER_BUFFERED(self) \ + ( (PyThread_acquire_lock(self->lock, 0) ? \ + 1 : _enter_buffered_busy(self)) \ + && (self->owner = PyThread_get_thread_ident(), 1) ) #define LEAVE_BUFFERED(self) \ - PyThread_release_lock(self->lock); + do { \ + self->owner = 0; \ + PyThread_release_lock(self->lock); \ + } while(0); + #else -#define ENTER_BUFFERED(self) +#define ENTER_BUFFERED(self) 1 #define LEAVE_BUFFERED(self) #endif @@ -444,7 +462,8 @@ buffered_close(buffered *self, PyObject *args) int r; CHECK_INITIALIZED(self) - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) + return NULL; r = buffered_closed(self); if (r < 0) @@ -465,7 +484,8 @@ buffered_close(buffered *self, PyObject *args) /* flush() will most probably re-take the lock, so drop it first */ LEAVE_BUFFERED(self) res = PyObject_CallMethodObjArgs((PyObject *)self, _PyIO_str_flush, NULL); - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) + return NULL; if (res == NULL) { goto end; } @@ -679,6 +699,7 @@ _buffered_init(buffered *self) PyErr_SetString(PyExc_RuntimeError, "can't allocate read lock"); return -1; } + self->owner = 0; #endif /* Find out whether buffer_size is a power of 2 */ /* XXX is this optimization useful? */ @@ -705,7 +726,8 @@ buffered_flush(buffered *self, PyObject *args) CHECK_INITIALIZED(self) CHECK_CLOSED(self, "flush of closed file") - ENTER_BUFFERED(self) + 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 @@ -732,7 +754,8 @@ buffered_peek(buffered *self, PyObject *args) return NULL; } - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) + return NULL; if (self->writable) { res = _bufferedwriter_flush_unlocked(self, 1); @@ -767,7 +790,8 @@ buffered_read(buffered *self, PyObject *args) if (n == -1) { /* The number of bytes is unspecified, read until the end of stream */ - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) + return NULL; res = _bufferedreader_read_all(self); LEAVE_BUFFERED(self) } @@ -775,7 +799,8 @@ buffered_read(buffered *self, PyObject *args) res = _bufferedreader_read_fast(self, n); if (res == Py_None) { Py_DECREF(res); - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) + return NULL; res = _bufferedreader_read_generic(self, n); LEAVE_BUFFERED(self) } @@ -803,7 +828,8 @@ buffered_read1(buffered *self, PyObject *args) if (n == 0) return PyBytes_FromStringAndSize(NULL, 0); - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) + return NULL; if (self->writable) { res = _bufferedwriter_flush_unlocked(self, 1); @@ -859,7 +885,8 @@ buffered_readinto(buffered *self, PyObject *args) /* TODO: use raw.readinto() instead! */ if (self->writable) { - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) + return NULL; res = _bufferedwriter_flush_unlocked(self, 0); LEAVE_BUFFERED(self) if (res == NULL) @@ -903,7 +930,8 @@ _buffered_readline(buffered *self, Py_ssize_t limit) goto end_unlocked; } - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) + goto end_unlocked; /* Now we try to get some more from the raw stream */ if (self->writable) { @@ -1053,7 +1081,8 @@ buffered_seek(buffered *self, PyObject *args) } } - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) + return NULL; /* Fallback: invoke raw seek() method and clear buffer */ if (self->writable) { @@ -1091,7 +1120,8 @@ buffered_truncate(buffered *self, PyObject *args) return NULL; } - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) + return NULL; if (self->writable) { res = _bufferedwriter_flush_unlocked(self, 0); @@ -1748,7 +1778,10 @@ bufferedwriter_write(buffered *self, PyObject *args) return NULL; } - ENTER_BUFFERED(self) + if (!ENTER_BUFFERED(self)) { + PyBuffer_Release(&buf); + return NULL; + } /* Fast path: the data to write can be fully buffered. */ if (!VALID_READ_BUFFER(self) && !VALID_WRITE_BUFFER(self)) { |