From c15c88c13dd67e320161760638778b17486100c1 Mon Sep 17 00:00:00 2001
From: Victor Stinner <victor.stinner@haypocalc.com>
Date: Thu, 1 Sep 2011 23:45:04 +0200
Subject: [PATCH] Issue #12494: Close pipes and kill process on error in
 subprocess functions

On error, call(), check_call(), check_output() and getstatusoutput() functions
of the subprocess module now kill the process, read its status (to avoid
zombis) and close pipes.
---
 Lib/subprocess.py | 56 ++++++++++++++++++++++++++++-------------------
 Misc/NEWS         |  4 ++++
 2 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index db64588697c..2c5c888910c 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -464,13 +464,13 @@ def call(*popenargs, timeout=None, **kwargs):
 
     retcode = call(["ls", "-l"])
     """
-    p = Popen(*popenargs, **kwargs)
-    try:
-        return p.wait(timeout=timeout)
-    except TimeoutExpired:
-        p.kill()
-        p.wait()
-        raise
+    with Popen(*popenargs, **kwargs) as p:
+        try:
+            return p.wait(timeout=timeout)
+        except:
+            p.kill()
+            p.wait()
+            raise
 
 
 def check_call(*popenargs, **kwargs):
@@ -514,16 +514,20 @@ def check_output(*popenargs, timeout=None, **kwargs):
     """
     if 'stdout' in kwargs:
         raise ValueError('stdout argument not allowed, it will be overridden.')
-    process = Popen(*popenargs, stdout=PIPE, **kwargs)
-    try:
-        output, unused_err = process.communicate(timeout=timeout)
-    except TimeoutExpired:
-        process.kill()
-        output, unused_err = process.communicate()
-        raise TimeoutExpired(process.args, timeout, output=output)
-    retcode = process.poll()
-    if retcode:
-        raise CalledProcessError(retcode, process.args, output=output)
+    with Popen(*popenargs, stdout=PIPE, **kwargs) as process:
+        try:
+            output, unused_err = process.communicate(timeout=timeout)
+        except TimeoutExpired:
+            process.kill()
+            output, unused_err = process.communicate()
+            raise TimeoutExpired(process.args, timeout, output=output)
+        except:
+            process.kill()
+            process.wait()
+            raise
+        retcode = process.poll()
+        if retcode:
+            raise CalledProcessError(retcode, process.args, output=output)
     return output
 
 
@@ -618,11 +622,19 @@ def getstatusoutput(cmd):
     >>> subprocess.getstatusoutput('/bin/junk')
     (256, 'sh: /bin/junk: not found')
     """
-    pipe = os.popen('{ ' + cmd + '; } 2>&1', 'r')
-    text = pipe.read()
-    sts = pipe.close()
-    if sts is None: sts = 0
-    if text[-1:] == '\n': text = text[:-1]
+    with os.popen('{ ' + cmd + '; } 2>&1', 'r') as pipe:
+        try:
+            text = pipe.read()
+            sts = pipe.close()
+        except:
+            process = pipe._proc
+            process.kill()
+            process.wait()
+            raise
+    if sts is None:
+        sts = 0
+    if text[-1:] == '\n':
+        text = text[:-1]
     return sts, text
 
 
diff --git a/Misc/NEWS b/Misc/NEWS
index 57a1237104c..01b72812bfd 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -271,6 +271,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #12494: On error, call(), check_call(), check_output() and
+  getstatusoutput() functions of the subprocess module now kill the process,
+  read its status (to avoid zombis) and close pipes.
+
 - Issue #12720: Expose low-level Linux extended file attribute functions in os.
 
 - Issue #10946: The distutils commands bdist_dumb, bdist_wininst and bdist_msi