From 16e026cc943460b83a08c058ba4e87c5abf36215 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Sat, 10 Apr 2010 22:43:05 +0000 Subject: [PATCH] Temporary commit of fix to issue #5380 (in order to watch buildbot response) --- Lib/test/test_io.py | 41 +++++++++++++++++++++++++++++++ Lib/test/test_posix.py | 24 +++++++++++++++--- Modules/_io/fileio.c | 55 ++++++++++++++++++++++++++++-------------- 3 files changed, 99 insertions(+), 21 deletions(-) diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 9ffe646ea9..accf0eb2cc 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -2394,6 +2394,47 @@ class MiscIOTest(unittest.TestCase): # baseline "io" module. self._check_abc_inheritance(io) + # Issue #5380: reading all available bytes from a pipe or a PTY when + # the other end has been closed. + + def check_pipe_func(self, pipe_func, buffered): + master_fd, slave_fd = pipe_func() + # Simulate a subprocess writing some data to the + # slave end of the pipe, and then exiting. + data = b'TEST DATA' + try: + os.write(slave_fd, data) + finally: + os.close(slave_fd) + with self.open(master_fd, "rb", buffering=-1 if buffered else 0) as f: + # Request more data than available + gotdata = f.read(len(data) + 1) + self.assertEqual(gotdata, data) + # Trying to read again returns an empty string + self.assertEqual(b'', f.read()) + self.assertEqual(b'', f.read(1)) + + def test_pipe_read_buffered(self): + if not hasattr(os, 'pipe'): + self.skipTest("os.pipe not available") + self.check_pipe_func(os.pipe, True) + + def test_pipe_read_raw(self): + if not hasattr(os, 'pipe'): + self.skipTest("os.pipe not available") + self.check_pipe_func(os.pipe, False) + + def test_openpty_read_buffered(self): + if not hasattr(os, 'openpty'): + self.skipTest("os.openpty not available") + self.check_pipe_func(os.openpty, True) + + def test_openpty_read_raw(self): + if not hasattr(os, 'openpty'): + self.skipTest("os.openpty not available") + self.check_pipe_func(os.openpty, False) + + class CMiscIOTest(MiscIOTest): io = io diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index afeb616ddc..3caf82493f 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -280,11 +280,29 @@ class PosixTester(unittest.TestCase): if hasattr(posix, 'strerror'): self.assertTrue(posix.strerror(0)) + def check_pipe_func(self, pipe_func): + master_fd, slave_fd = pipe_func() + try: + # Simulate a subprocess writing some data to the + # slave end of the pipe, and then exiting. + data = b'TEST DATA' + try: + os.write(slave_fd, data) + finally: + os.close(slave_fd) + # Request more data than available + gotdata = os.read(master_fd, len(data) + 1) + self.assertEqual(gotdata, data) + finally: + os.close(master_fd) + def test_pipe(self): if hasattr(posix, 'pipe'): - reader, writer = posix.pipe() - os.close(reader) - os.close(writer) + self.check_pipe_func(posix.pipe) + + def test_openpty(self): + if hasattr(posix, 'openpty'): + self.check_pipe_func(posix.openpty) def test_tempnam(self): if hasattr(posix, 'tempnam'): diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index ec320f79b9..8f174a7edb 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -464,6 +464,34 @@ fileio_seekable(fileio *self) return PyBool_FromLong((long) self->seekable); } +static Py_ssize_t +internal_read(int fd, void *buf, size_t count) +{ + Py_ssize_t n; + + Py_BEGIN_ALLOW_THREADS + errno = 0; + n = read(fd, buf, count); +#ifdef EIO + /* Issue #5380: when reading past the end of a pipe created by + openpty(), EIO can be set. Make it an EOF instead, so that + the normal technique of testing for an empty string can be used. + */ + if (n == -1 && errno == EIO) { + if (isatty(fd)) { + n = 0; + errno = 0; + } + else { + /* isatty() set errno, restore its value */ + errno = EIO; + } + } +#endif + Py_END_ALLOW_THREADS + return n; +} + static PyObject * fileio_readinto(fileio *self, PyObject *args) { @@ -478,12 +506,9 @@ fileio_readinto(fileio *self, PyObject *args) if (!PyArg_ParseTuple(args, "w*", &pbuf)) return NULL; - if (_PyVerify_fd(self->fd)) { - Py_BEGIN_ALLOW_THREADS - errno = 0; - n = read(self->fd, pbuf.buf, pbuf.len); - Py_END_ALLOW_THREADS - } else + if (_PyVerify_fd(self->fd)) + n = internal_read(self->fd, pbuf.buf, pbuf.len); + else n = -1; PyBuffer_Release(&pbuf); if (n < 0) { @@ -560,12 +585,9 @@ fileio_readall(fileio *self) break; } } - Py_BEGIN_ALLOW_THREADS - errno = 0; - n = read(self->fd, - PyBytes_AS_STRING(result) + total, - newsize - total); - Py_END_ALLOW_THREADS + n = internal_read(self->fd, + PyBytes_AS_STRING(result) + total, + newsize - total); if (n == 0) break; if (n < 0) { @@ -617,12 +639,9 @@ fileio_read(fileio *self, PyObject *args) return NULL; ptr = PyBytes_AS_STRING(bytes); - if (_PyVerify_fd(self->fd)) { - Py_BEGIN_ALLOW_THREADS - errno = 0; - n = read(self->fd, ptr, size); - Py_END_ALLOW_THREADS - } else + if (_PyVerify_fd(self->fd)) + n = internal_read(self->fd, ptr, size); + else n = -1; if (n < 0) { -- 2.40.0