From 33cf4629a493a7adb9fdc3655d4639bf6c4014dc Mon Sep 17 00:00:00 2001 From: Alexey Izbyshev Date: Sun, 23 Sep 2018 19:44:23 +0300 Subject: [PATCH] bpo-34780: Fix potential hang during stdin initialization on Windows On Windows, if there is an outstanding operation on a synchronous pipe handle (e.g., ReadFile()), many other operations will block until the first one completes. This is true even for operations that normally complete immediately (PeekNamedPipe(), SetFilePointer(), etc.). Python performs fstat(fd) during file object initialization to check for attempts to open a directory, which is forbidden. If fd refers to a pipe, msvcrt calls PeekNamedPipe() to fill in st_size member of struct stat, potentially hanging the interpreter before execution of user code when stdin object is initialized in the conditions described above. fstat() on Windows can't distinguish between a file and a directory because it relies on GetFileType(), which returns FILE_TYPE_DISK in both cases. Therefore, skipping the no-op check avoids the hang. --- Lib/test/test_io.py | 39 +++++++++++++++++++ .../2018-09-23-23-02-46.bpo-34780.PIJ3OU.rst | 2 + Modules/posixmodule.c | 3 +- Objects/fileobject.c | 3 +- 4 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Windows/2018-09-23-23-02-46.bpo-34780.PIJ3OU.rst diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 26c5dfe9261b7f6..890f4d77cad1cde 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -32,6 +32,8 @@ import warnings import abc import signal +import subprocess +import textwrap import errno from itertools import cycle, count from collections import deque @@ -3132,6 +3134,43 @@ def _test_nonblock_pipe_write(self, bufsize): self.assertTrue(wf.closed) self.assertTrue(rf.closed) + # bpo-34780 + @unittest.skipUnless(sys.platform == 'win32', 'Win32-specific test') + def test_stdin_init_hang(self): + r, w = os.pipe() + self.addCleanup(os.close, r) + self.addCleanup(os.close, w) + code = textwrap.dedent(""" + import sys + sys.stdout.write('OK') + sys.stdout.flush() + sys.stdin.read() + """) + p = subprocess.Popen([sys.executable, '-c', code], + stdin=r, stdout=subprocess.PIPE, bufsize=0) + try: + # Synchronize to increase chances that p is blocked in ReadFile() + # when p2 is started. + self.assertEqual(p.stdout.read(2), b'OK') + p.stdout.close() + p2 = subprocess.Popen([sys.executable, '-c', ''], stdin=r) + try: + start = time.time() + while time.time() - start < 10: + exit_code = p2.poll() + if exit_code is not None: + self.assertEqual(exit_code, 0) + break + time.sleep(0.1) + else: + self.fail('Child interpreter appears to hang') + finally: + p2.kill() + p2.wait() + finally: + p.kill() + p.wait() + class CMiscIOTest(MiscIOTest): io = io shutdown_error = "RuntimeError: could not find io module state" diff --git a/Misc/NEWS.d/next/Windows/2018-09-23-23-02-46.bpo-34780.PIJ3OU.rst b/Misc/NEWS.d/next/Windows/2018-09-23-23-02-46.bpo-34780.PIJ3OU.rst new file mode 100644 index 000000000000000..1adb8580724c2c3 --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2018-09-23-23-02-46.bpo-34780.PIJ3OU.rst @@ -0,0 +1,2 @@ +Fix hang on startup if stdin refers to a pipe with an outstanding concurrent +operation on Windows. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 7a1a6945c102eb1..a88d974d34e92fe 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -6915,7 +6915,8 @@ posix_fdopen(PyObject *self, PyObject *args) PyMem_FREE(mode); return posix_error(); } -#if defined(HAVE_FSTAT) && defined(S_IFDIR) && defined(EISDIR) +#if !defined(MS_WINDOWS) && defined(HAVE_FSTAT) && defined(S_IFDIR) && \ + defined(EISDIR) { struct stat buf; const char *msg; diff --git a/Objects/fileobject.c b/Objects/fileobject.c index b524f09e0a396ed..b5a0aec1f848917 100644 --- a/Objects/fileobject.c +++ b/Objects/fileobject.c @@ -119,7 +119,8 @@ file_PyObject_Print(PyObject *op, PyFileObject *f, int flags) static PyFileObject* dircheck(PyFileObject* f) { -#if defined(HAVE_FSTAT) && defined(S_IFDIR) && defined(EISDIR) +#if !defined(MS_WINDOWS) && defined(HAVE_FSTAT) && defined(S_IFDIR) && \ + defined(EISDIR) struct stat buf; int res; if (f->f_fp == NULL)