cpython: 19a6bef57490 (original) (raw)

Mercurial > cpython

changeset 77673:19a6bef57490

Fixes issue #12268: File readline, readlines and read() or readall() methods no longer lose data when an underlying read system call is interrupted. IOError is no longer raised due to a read system call returning EINTR from within these methods. [#12268]

Gregory P. Smith greg@krypto.org
date	Sun, 24 Jun 2012 00:23:47 -0700
parents	02bcfb7aa2ca(current diff)781b95159954(diff)
children	ba6ca42919dd
files	Lib/test/test_io.py Misc/NEWS Modules/_io/_iomodule.h Modules/_io/bufferedio.c Modules/_io/fileio.c Modules/_io/iobase.c Modules/_io/textio.c
diffstat	8 files changed, 295 insertions(+), 15 deletions(-)[+] [-] Lib/test/test_file_eintr.py 236 Lib/test/test_io.py 10 Misc/NEWS 5 Modules/_io/_iomodule.h 5 Modules/_io/bufferedio.c 8 Modules/_io/fileio.c 7 Modules/_io/iobase.c 23 Modules/_io/textio.c 16

line wrap: on

line diff

new file mode 100644 --- /dev/null +++ b/Lib/test/test_file_eintr.py @@ -0,0 +1,236 @@ +# Written to test interrupted system calls interfering with our many buffered +# IO implementations. http://bugs.python.org/issue12268[](#l1.6) +# +# It was suggested that this code could be merged into test_io and the tests +# made to work using the same method as the existing signal tests in test_io. +# I was unable to get single process tests using alarm or setitimer that way +# to reproduce the EINTR problems. This process based test suite reproduces +# the problems prior to the issue12268 patch reliably on Linux and OSX. +# - gregory.p.smith + +import os +import select +import signal +import subprocess +import sys +from test.support import run_unittest +import time +import unittest + +# Test import all of the things we're about to try testing up front. +from _io import FileIO + + +@unittest.skipUnless(os.name == 'posix', 'tests requires a posix system.') +class TestFileIOSignalInterrupt(unittest.TestCase):

def setUp(self):
```
   self._process = None[](#l1.31)
```

def tearDown(self):

   if self._process and self._process.poll() is None:[](#l1.34)

```
       try:[](#l1.35)
```

           self._process.kill()[](#l1.36)

```
       except OSError:[](#l1.37)
```
```
           pass[](#l1.38)
```

def _generate_infile_setup_code(self):

   """Returns the infile = ... line of code for the reader process.[](#l1.41)

   subclasseses should override this to test different IO objects.[](#l1.43)

```
   """[](#l1.44)
```
```
   return ('import _io ;'[](#l1.45)
```

           'infile = _io.FileIO(sys.stdin.fileno(), "rb")')[](#l1.46)

def fail_with_process_info(self, why, stdout=b'', stderr=b'',

                          communicate=True):[](#l1.49)

   """A common way to cleanup and fail with useful debug output.[](#l1.50)

   Kills the process if it is still running, collects remaining output[](#l1.52)

   and fails the test with an error message including the output.[](#l1.53)

```
   Args:[](#l1.55)
```

       why: Text to go after "Error from IO process" in the message.[](#l1.56)

       stdout, stderr: standard output and error from the process so[](#l1.57)

           far to include in the error message.[](#l1.58)

       communicate: bool, when True we call communicate() on the process[](#l1.59)

           after killing it to gather additional output.[](#l1.60)

```
   """[](#l1.61)
```

   if self._process.poll() is None:[](#l1.62)

       time.sleep(0.1)  # give it time to finish printing the error.[](#l1.63)

```
       try:[](#l1.64)
```

           self._process.terminate()  # Ensure it dies.[](#l1.65)

```
       except OSError:[](#l1.66)
```
```
           pass[](#l1.67)
```
```
   if communicate:[](#l1.68)
```

       stdout_end, stderr_end = self._process.communicate()[](#l1.69)

```
       stdout += stdout_end[](#l1.70)
```
```
       stderr += stderr_end[](#l1.71)
```

   self.fail('Error from IO process %s:\nSTDOUT:\n%sSTDERR:\n%s\n' %[](#l1.72)

             (why, stdout.decode(), stderr.decode()))[](#l1.73)

def _test_reading(self, data_to_write, read_and_verify_code):

   """Generic buffered read method test harness to validate EINTR behavior.[](#l1.76)

   Also validates that Python signal handlers are run during the read.[](#l1.78)

```
   Args:[](#l1.80)
```

       data_to_write: String to write to the child process for reading[](#l1.81)

           before sending it a signal, confirming the signal was handled,[](#l1.82)

           writing a final newline and closing the infile pipe.[](#l1.83)

       read_and_verify_code: Single "line" of code to read from a file[](#l1.84)

           object named 'infile' and validate the result.  This will be[](#l1.85)

           executed as part of a python subprocess fed data_to_write.[](#l1.86)

```
   """[](#l1.87)
```

   infile_setup_code = self._generate_infile_setup_code()[](#l1.88)

   # Total pipe IO in this function is smaller than the minimum posix OS[](#l1.89)

   # pipe buffer size of 512 bytes.  No writer should block.[](#l1.90)

   assert len(data_to_write) < 512, 'data_to_write must fit in pipe buf.'[](#l1.91)

   # Start a subprocess to call our read method while handling a signal.[](#l1.93)

   self._process = subprocess.Popen([](#l1.94)

           [sys.executable, '-u', '-c',[](#l1.95)

            'import signal, sys ;'[](#l1.96)

            'signal.signal(signal.SIGINT, '[](#l1.97)

                          'lambda s, f: sys.stderr.write("$\\n")) ;'[](#l1.98)

            + infile_setup_code + ' ;' +[](#l1.99)

            'sys.stderr.write("Worm Sign!\\n") ;'[](#l1.100)

            + read_and_verify_code + ' ;' +[](#l1.101)

```
            'infile.close()'[](#l1.102)
```
```
           ],[](#l1.103)
```

           stdin=subprocess.PIPE, stdout=subprocess.PIPE,[](#l1.104)

           stderr=subprocess.PIPE)[](#l1.105)

   # Wait for the signal handler to be installed.[](#l1.107)

   worm_sign = self._process.stderr.read(len(b'Worm Sign!\n'))[](#l1.108)

   if worm_sign != b'Worm Sign!\n':  # See also, Dune by Frank Herbert.[](#l1.109)

       self.fail_with_process_info('while awaiting a sign',[](#l1.110)

                                   stderr=worm_sign)[](#l1.111)

   self._process.stdin.write(data_to_write)[](#l1.112)

```
   signals_sent = 0[](#l1.114)
```
```
   rlist = [][](#l1.115)
```

   # We don't know when the read_and_verify_code in our child is actually[](#l1.116)

   # executing within the read system call we want to interrupt.  This[](#l1.117)

   # loop waits for a bit before sending the first signal to increase[](#l1.118)

   # the likelihood of that.  Implementations without correct EINTR[](#l1.119)

   # and signal handling usually fail this test.[](#l1.120)

```
   while not rlist:[](#l1.121)
```

       rlist, _, _ = select.select([self._process.stderr], (), (), 0.05)[](#l1.122)

       self._process.send_signal(signal.SIGINT)[](#l1.123)

```
       signals_sent += 1[](#l1.124)
```

       if signals_sent > 200:[](#l1.125)

           self._process.kill()[](#l1.126)

           self.fail('reader process failed to handle our signals.')[](#l1.127)

   # This assumes anything unexpected that writes to stderr will also[](#l1.128)

   # write a newline.  That is true of the traceback printing code.[](#l1.129)

   signal_line = self._process.stderr.readline()[](#l1.130)

```
   if signal_line != b'$\n':[](#l1.131)
```

       self.fail_with_process_info('while awaiting signal',[](#l1.132)

                                   stderr=signal_line)[](#l1.133)

   # We append a newline to our input so that a readline call can[](#l1.135)

   # end on its own before the EOF is seen and so that we're testing[](#l1.136)

   # the read call that was interrupted by a signal before the end of[](#l1.137)

   # the data stream has been reached.[](#l1.138)

   stdout, stderr = self._process.communicate(input=b'\n')[](#l1.139)

   if self._process.returncode:[](#l1.140)

       self.fail_with_process_info([](#l1.141)

               'exited rc=%d' % self._process.returncode,[](#l1.142)

               stdout, stderr, communicate=False)[](#l1.143)

```
   # PASS![](#l1.144)
```

String format for the read_and_verify_code used by read methods.
_READING_CODE_TEMPLATE = (

       'got = infile.{read_method_name}() ;'[](#l1.148)

       'expected = {expected!r} ;'[](#l1.149)

       'assert got == expected, ('[](#l1.150)

               '"{read_method_name} returned wrong data.\\n"'[](#l1.151)

               '"got data %r\\nexpected %r" % (got, expected))'[](#l1.152)

```
       )[](#l1.153)
```

def test_readline(self):

   """readline() must handle signals and not lose data."""[](#l1.156)

```
   self._test_reading([](#l1.157)
```

           data_to_write=b'hello, world!',[](#l1.158)

           read_and_verify_code=self._READING_CODE_TEMPLATE.format([](#l1.159)

                   read_method_name='readline',[](#l1.160)

                   expected=b'hello, world!\n'))[](#l1.161)

def test_readlines(self):

   """readlines() must handle signals and not lose data."""[](#l1.164)

```
   self._test_reading([](#l1.165)
```

           data_to_write=b'hello\nworld!',[](#l1.166)

           read_and_verify_code=self._READING_CODE_TEMPLATE.format([](#l1.167)

                   read_method_name='readlines',[](#l1.168)

                   expected=[b'hello\n', b'world!\n']))[](#l1.169)

def test_readall(self):

   """readall() must handle signals and not lose data."""[](#l1.172)

```
   self._test_reading([](#l1.173)
```

           data_to_write=b'hello\nworld!',[](#l1.174)

           read_and_verify_code=self._READING_CODE_TEMPLATE.format([](#l1.175)

                   read_method_name='readall',[](#l1.176)

                   expected=b'hello\nworld!\n'))[](#l1.177)

   # read() is the same thing as readall().[](#l1.178)

```
   self._test_reading([](#l1.179)
```

           data_to_write=b'hello\nworld!',[](#l1.180)

           read_and_verify_code=self._READING_CODE_TEMPLATE.format([](#l1.181)

                   read_method_name='read',[](#l1.182)

                   expected=b'hello\nworld!\n'))[](#l1.183)

+ + +class TestBufferedIOSignalInterrupt(TestFileIOSignalInterrupt):

def _generate_infile_setup_code(self):

   """Returns the infile = ... line of code to make a BufferedReader."""[](#l1.188)

   return ('infile = open(sys.stdin.fileno(), "rb") ;'[](#l1.189)

           'import _io ;assert isinstance(infile, _io.BufferedReader)')[](#l1.190)

def test_readall(self):

   """BufferedReader.read() must handle signals and not lose data."""[](#l1.193)

```
   self._test_reading([](#l1.194)
```

           data_to_write=b'hello\nworld!',[](#l1.195)

           read_and_verify_code=self._READING_CODE_TEMPLATE.format([](#l1.196)

                   read_method_name='read',[](#l1.197)

                   expected=b'hello\nworld!\n'))[](#l1.198)

+ + +class TestTextIOSignalInterrupt(TestFileIOSignalInterrupt):

def _generate_infile_setup_code(self):

   """Returns the infile = ... line of code to make a TextIOWrapper."""[](#l1.203)

   return ('infile = open(sys.stdin.fileno(), "rt", newline=None) ;'[](#l1.204)

           'import _io ;assert isinstance(infile, _io.TextIOWrapper)')[](#l1.205)

def test_readline(self):

   """readline() must handle signals and not lose data."""[](#l1.208)

```
   self._test_reading([](#l1.209)
```

           data_to_write=b'hello, world!',[](#l1.210)

           read_and_verify_code=self._READING_CODE_TEMPLATE.format([](#l1.211)

                   read_method_name='readline',[](#l1.212)

                   expected='hello, world!\n'))[](#l1.213)

def test_readlines(self):

   """readlines() must handle signals and not lose data."""[](#l1.216)

```
   self._test_reading([](#l1.217)
```

           data_to_write=b'hello\r\nworld!',[](#l1.218)

           read_and_verify_code=self._READING_CODE_TEMPLATE.format([](#l1.219)

                   read_method_name='readlines',[](#l1.220)

                   expected=['hello\n', 'world!\n']))[](#l1.221)

def test_readall(self):

   """read() must handle signals and not lose data."""[](#l1.224)

```
   self._test_reading([](#l1.225)
```

           data_to_write=b'hello\nworld!',[](#l1.226)

           read_and_verify_code=self._READING_CODE_TEMPLATE.format([](#l1.227)

                   read_method_name='read',[](#l1.228)

                   expected="hello\nworld!\n"))[](#l1.229)

+ + +def test_main():

test_cases = [

       tc for tc in globals().values()[](#l1.234)

       if isinstance(tc, type) and issubclass(tc, unittest.TestCase)][](#l1.235)

run_unittest(*test_cases)

+ + +if name == 'main':

test_main()

--- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -2912,7 +2912,7 @@ class SignalsTest(unittest.TestCase): try: wio = self.io.open(w, **fdopen_kwargs) t.start()

```
       signal.alarm(1)[](#l2.7)
```

       signal.setitimer(signal.ITIMER_REAL, 0.1)[](#l2.8)
       # Fill the pipe enough that the write will be blocking.[](#l2.9)
       # It will be interrupted by the timer armed above.  Since the[](#l2.10)
       # other thread has read one byte, the low-level write will[](#l2.11)

@@ -2957,7 +2957,7 @@ class SignalsTest(unittest.TestCase): r, w = os.pipe() wio = self.io.open(w, **fdopen_kwargs) try:

```
       signal.alarm(1)[](#l2.16)
```

       signal.setitimer(signal.ITIMER_REAL, 0.1)[](#l2.17)
       # Either the reentrant call to wio.write() fails with RuntimeError,[](#l2.18)
       # or the signal handler raises ZeroDivisionError.[](#l2.19)
       with self.assertRaises((ZeroDivisionError, RuntimeError)) as cm:[](#l2.20)

@@ -2992,7 +2992,7 @@ class SignalsTest(unittest.TestCase): try: rio = self.io.open(r, **fdopen_kwargs) os.write(w, b"foo")

```
       signal.alarm(1)[](#l2.25)
```

       signal.setitimer(signal.ITIMER_REAL, 0.1)[](#l2.26)
       # Expected behaviour:[](#l2.27)
       # - first raw read() returns partial b"foo"[](#l2.28)
       # - second raw read() returns EINTR[](#l2.29)

@@ -3036,13 +3036,13 @@ class SignalsTest(unittest.TestCase): t.daemon = True def alarm1(sig, frame): signal.signal(signal.SIGALRM, alarm2)

```
       signal.alarm(1)[](#l2.34)
```

       signal.setitimer(signal.ITIMER_REAL, 0.1)[](#l2.35)
   def alarm2(sig, frame):[](#l2.36)
       t.start()[](#l2.37)
   signal.signal(signal.SIGALRM, alarm1)[](#l2.38)
   try:[](#l2.39)
       wio = self.io.open(w, **fdopen_kwargs)[](#l2.40)

```
       signal.alarm(1)[](#l2.41)
```

       signal.setitimer(signal.ITIMER_REAL, 0.1)[](#l2.42)
       # Expected behaviour:[](#l2.43)
       # - first raw write() is partial (because of the limited pipe buffer[](#l2.44)
       #   and the first alarm)[](#l2.45)

--- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,11 @@ What's New in Python 3.3.0 Beta 1? Core and Builtins ----------------- +- Issue #12268: File readline, readlines and read() or readall() methods

no longer lose data when an underlying read system call is interrupted.
IOError is no longer raised due to a read system call returning EINTR
from within these methods. +

Issue #11626: Add _SizeT functions to stable ABI.
Issue #15146: Add PyType_FromSpecWithBases. Patch by Robin Schreiber.

--- a/Modules/_io/_iomodule.h +++ b/Modules/_io/_iomodule.h @@ -57,6 +57,11 @@ extern Py_ssize_t _PyIO_find_line_ending int translated, int universal, PyObject *readnl, int kind, char *start, char *end, Py_ssize_t consumed); +/ Return 1 if an EnvironmentError with errno == EINTR is set (and then

clears the error indicator), 0 otherwise.
Should only be called when PyErr_Occurred() is true. +*/ +extern int _PyIO_trap_eintr(void);

#define DEFAULT_BUFFER_SIZE (8 * 1024) /* bytes */

--- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -746,8 +746,8 @@ static int clears the error indicator), 0 otherwise. Should only be called when PyErr_Occurred() is true. */ -static int -_trap_eintr(void) +int +_PyIO_trap_eintr(void) { static PyObject *eintr_int = NULL; PyObject *typ, *val, *tb; @@ -1396,7 +1396,7 @@ static Py_ssize_t */ do { res = PyObject_CallMethodObjArgs(self->raw, _PyIO_str_readinto, memobj, NULL);

} while (res == NULL && _trap_eintr());

} while (res == NULL && _PyIO_trap_eintr()); Py_DECREF(memobj); if (res == NULL) return -1;

@@ -1850,7 +1850,7 @@ static Py_ssize_t errno = 0; res = PyObject_CallMethodObjArgs(self->raw, _PyIO_str_write, memobj, NULL); errnum = errno;

} while (res == NULL && _trap_eintr());

} while (res == NULL && _PyIO_trap_eintr()); Py_DECREF(memobj); if (res == NULL) return -1;

--- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -670,6 +670,13 @@ fileio_readall(fileio *self) if (n == 0) break; if (n < 0) {

```
       if (errno == EINTR) {[](#l6.7)
```

           if (PyErr_CheckSignals()) {[](#l6.8)

               Py_DECREF(result);[](#l6.9)

```
               return NULL;[](#l6.10)
```
```
           }[](#l6.11)
```
```
           continue;[](#l6.12)
```

       }[](#l6.13)
       if (total > 0)[](#l6.14)
           break;[](#l6.15)
       if (errno == EAGAIN) {[](#l6.16)

--- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -474,11 +474,15 @@ iobase_readline(PyObject *self, PyObject PyObject *b; if (has_peek) {

       _Py_IDENTIFIER(peek);[](#l7.7)
       PyObject *readahead = _PyObject_CallMethodId(self, &PyId_peek, "i", 1);[](#l7.8)

```
       if (readahead == NULL)[](#l7.10)
```

       if (readahead == NULL) {[](#l7.11)

           /* NOTE: PyErr_SetFromErrno() calls PyErr_CheckSignals()[](#l7.12)

              when EINTR occurs so we needn't do it ourselves. */[](#l7.13)

           if (_PyIO_trap_eintr()) {[](#l7.14)

```
               continue;[](#l7.15)
```

           }[](#l7.16)
           goto fail;[](#l7.17)

       }[](#l7.18)
       if (!PyBytes_Check(readahead)) {[](#l7.19)
           PyErr_Format(PyExc_IOError,[](#l7.20)
                        "peek() should have returned a bytes object, "[](#l7.21)

@@ -511,8 +515,14 @@ iobase_readline(PyObject *self, PyObject } b = _PyObject_CallMethodId(self, &PyId_read, "n", nreadahead);

```
   if (b == NULL)[](#l7.26)
```

```
   if (b == NULL) {[](#l7.27)
```

       /* NOTE: PyErr_SetFromErrno() calls PyErr_CheckSignals()[](#l7.28)

          when EINTR occurs so we needn't do it ourselves. */[](#l7.29)

       if (_PyIO_trap_eintr()) {[](#l7.30)

```
           continue;[](#l7.31)
```

       }[](#l7.32)
       goto fail;[](#l7.33)

   }[](#l7.34)
   if (!PyBytes_Check(b)) {[](#l7.35)
       PyErr_Format(PyExc_IOError,[](#l7.36)
                    "read() should have returned a bytes object, "[](#l7.37)

@@ -827,6 +837,11 @@ rawiobase_readall(PyObject *self, PyObje PyObject *data = _PyObject_CallMethodId(self, &PyId_read, "i", DEFAULT_BUFFER_SIZE); if (!data) {

       /* NOTE: PyErr_SetFromErrno() calls PyErr_CheckSignals()[](#l7.42)

          when EINTR occurs so we needn't do it ourselves. */[](#l7.43)

       if (_PyIO_trap_eintr()) {[](#l7.44)

```
           continue;[](#l7.45)
```

       }[](#l7.46)
       Py_DECREF(chunks);[](#l7.47)
       return NULL;[](#l7.48)
   }[](#l7.49)

--- a/Modules/_io/textio.c +++ b/Modules/_io/textio.c @@ -1560,8 +1560,14 @@ textiowrapper_read(textio self, PyObjec / Keep reading chunks until we have n characters to return */ while (remaining > 0) { res = textiowrapper_read_chunk(self, remaining);

```
       if (res < 0)[](#l8.7)
```

```
       if (res < 0) {[](#l8.8)
```

           /* NOTE: PyErr_SetFromErrno() calls PyErr_CheckSignals()[](#l8.9)

              when EINTR occurs so we needn't do it ourselves. */[](#l8.10)

           if (_PyIO_trap_eintr()) {[](#l8.11)

```
               continue;[](#l8.12)
```

           }[](#l8.13)
           goto fail;[](#l8.14)

       }[](#l8.15)
       if (res == 0)  /* EOF */[](#l8.16)
           break;[](#l8.17)
       if (chunks == NULL) {[](#l8.18)

@@ -1728,8 +1734,14 @@ static PyObject * while (!self->decoded_chars || !PyUnicode_GET_LENGTH(self->decoded_chars)) { res = textiowrapper_read_chunk(self, 0);

```
       if (res < 0)[](#l8.23)
```

```
       if (res < 0) {[](#l8.24)
```

           /* NOTE: PyErr_SetFromErrno() calls PyErr_CheckSignals()[](#l8.25)

              when EINTR occurs so we needn't do it ourselves. */[](#l8.26)

           if (_PyIO_trap_eintr()) {[](#l8.27)

```
               continue;[](#l8.28)
```

           }[](#l8.29)
           goto error;[](#l8.30)

       }[](#l8.31)
       if (res == 0)[](#l8.32)
           break;[](#l8.33)
   }[](#l8.34)

cpython: 19a6bef57490 (original) (raw)

Mercurial > cpython

changeset 77673:19a6bef57490

String format for the read_and_verify_code used by read methods.