From d3ae95e1e945ed20297e1c38ba43a18b7a868ab6 Mon Sep 17 00:00:00 2001
From: Alex Rebert <alex@forallsecure.com>
Date: Wed, 22 Jan 2020 18:28:31 -0500
Subject: [PATCH] bpo-35182: fix communicate() crash after child closes its
 pipes (GH-17020) (GH-18117)

When communicate() is called in a loop, it crashes when the child process
has already closed any piped standard stream, but still continues to be running

Co-authored-by: Andriy Maletsky <andriy.maletsky@gmail.com>
---
 Lib/subprocess.py                                     |  4 ++--
 Lib/test/test_subprocess.py                           | 11 +++++++++++
 .../Library/2019-10-31-19-23-25.bpo-35182.hzeNl9.rst  |  3 +++
 3 files changed, 16 insertions(+), 2 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Library/2019-10-31-19-23-25.bpo-35182.hzeNl9.rst

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 79dffd349a3..26a1e69bd38 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1983,9 +1983,9 @@ class Popen(object):
             with _PopenSelector() as selector:
                 if self.stdin and input:
                     selector.register(self.stdin, selectors.EVENT_WRITE)
-                if self.stdout:
+                if self.stdout and not self.stdout.closed:
                     selector.register(self.stdout, selectors.EVENT_READ)
-                if self.stderr:
+                if self.stderr and not self.stderr.closed:
                     selector.register(self.stderr, selectors.EVENT_READ)
 
                 while selector.get_map():
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index f1fb93455dd..2bbdbaef84e 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -3145,6 +3145,17 @@ class POSIXProcessTestCase(BaseTestCase):
         # so Popen failed to read it and uses a default returncode instead.
         self.assertIsNotNone(proc.returncode)
 
+    def test_communicate_repeated_call_after_stdout_close(self):
+        proc = subprocess.Popen([sys.executable, '-c',
+                                 'import os, time; os.close(1), time.sleep(2)'],
+                                stdout=subprocess.PIPE)
+        while True:
+            try:
+                proc.communicate(timeout=0.1)
+                return
+            except subprocess.TimeoutExpired:
+                pass
+
 
 @unittest.skipUnless(mswindows, "Windows specific tests")
 class Win32ProcessTestCase(BaseTestCase):
diff --git a/Misc/NEWS.d/next/Library/2019-10-31-19-23-25.bpo-35182.hzeNl9.rst b/Misc/NEWS.d/next/Library/2019-10-31-19-23-25.bpo-35182.hzeNl9.rst
new file mode 100644
index 00000000000..9438cd8f9fd
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-10-31-19-23-25.bpo-35182.hzeNl9.rst
@@ -0,0 +1,3 @@
+Fixed :func:`Popen.communicate` subsequent call crash when the child process
+has already closed any piped standard stream, but still continues to be
+running. Patch by Andriy Maletsky.