From d23047b62c6f885def9020bd9b304110f9b9c52d Mon Sep 17 00:00:00 2001
From: "Gregory P. Smith" <greg@mad-scientist.com>
Date: Sat, 4 Dec 2010 09:10:44 +0000
Subject: [PATCH] issue7213 + issue2320: Cause a DeprecationWarning if the
 close_fds argument is not passed to subprocess.Popen as the default value
 will be changing in a future Python to the safer and more often desired value
 of True.

DeprecationWarnings that show up in a lot of existing code are controversial
and have caused pain in the past.  I'd like to leave this on for 3.2 beta1 and
see how things go.  We can remove the warning if it is deemed too noisy during
any betas.  (case study: the md5 and sha module DeprecationWarnings are loathed
around the world as those modules were never going to be removed in 2.x and
2to3 has a fixer for code that uses them)
---
 Doc/library/subprocess.rst  |  7 ++++++-
 Lib/subprocess.py           | 13 +++++++++++--
 Lib/test/test_subprocess.py | 32 +++++++++++++++++++++++++++++++-
 3 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst
index e0bb1635e34..a346d98cf0d 100644
--- a/Doc/library/subprocess.rst
+++ b/Doc/library/subprocess.rst
@@ -153,10 +153,15 @@ This module defines one class called :class:`Popen`:
 
    If *close_fds* is true, all file descriptors except :const:`0`, :const:`1` and
    :const:`2` will be closed before the child process is executed. (Unix only).
-   Or, on Windows, if *close_fds* is true then no handles will be inherited by the
+   The recommended value for this argument is True.
+   On Windows, if *close_fds* is true then no handles will be inherited by the
    child process.  Note that on Windows, you cannot set *close_fds* to true and
    also redirect the standard handles by setting *stdin*, *stdout* or *stderr*.
 
+.. versionchanged:: 3.2
+   Callers should always specify a *close_fds* to avoid a DeprecationWarning.
+   The default value for this argument will be changing in Python 3.3.
+
    If *shell* is :const:`True`, the specified command will be executed through the
    shell.
 
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index bdbcc6b6f24..ca670127f00 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -339,6 +339,7 @@ import traceback
 import gc
 import signal
 import builtins
+import warnings
 
 # Exception classes used by this module.
 class CalledProcessError(Exception):
@@ -378,7 +379,6 @@ else:
         import _posixsubprocess
     except ImportError:
         _posixsubprocess = None
-        import warnings
         warnings.warn("The _posixsubprocess module is not being used. "
                       "Child process reliability may suffer if your "
                       "program uses threads.", RuntimeWarning)
@@ -605,7 +605,7 @@ def getoutput(cmd):
 class Popen(object):
     def __init__(self, args, bufsize=0, executable=None,
                  stdin=None, stdout=None, stderr=None,
-                 preexec_fn=None, close_fds=False, shell=False,
+                 preexec_fn=None, close_fds=None, shell=False,
                  cwd=None, env=None, universal_newlines=False,
                  startupinfo=None, creationflags=0,
                  restore_signals=True, start_new_session=False):
@@ -618,6 +618,15 @@ class Popen(object):
         if not isinstance(bufsize, int):
             raise TypeError("bufsize must be an integer")
 
+        if close_fds is None:
+            # Notification for http://bugs.python.org/issue7213 & issue2320
+            warnings.warn(
+                    'The close_fds parameter was not specified.  Its default'
+                    ' will change from False to True in a future Python'
+                    ' version.  Most users should set it to True.  Please'
+                    ' update your code explicitly set close_fds.',
+                    DeprecationWarning)
+
         if mswindows:
             if preexec_fn is not None:
                 raise ValueError("preexec_fn is not supported on Windows "
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index ec1e18688cb..8759941a026 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -9,6 +9,7 @@ import tempfile
 import time
 import re
 import sysconfig
+import warnings
 try:
     import gc
 except ImportError:
@@ -57,6 +58,34 @@ class BaseTestCase(unittest.TestCase):
         self.assertEqual(actual, expected, msg)
 
 
+class DeprecationWarningTests(BaseTestCase):
+    def setUp(self):
+        BaseTestCase.setUp(self)
+        self._saved_warn = warnings.warn
+        self._warn_calls = []
+        warnings.warn = self._record_warn
+
+    def tearDown(self):
+        warnings.warn = self._saved_warn
+        BaseTestCase.tearDown(self)
+
+    def _record_warn(self, *args):
+        """A warnings.warn function that records calls."""
+        self._warn_calls.append(args)
+        self._saved_warn(*args)
+
+    def testCloseFdsWarning(self):
+        quick_process = [sys.executable, "-c", "import sys; sys.exit(0)"]
+        subprocess.call(quick_process, close_fds=True)
+        self.assertEqual([], self._warn_calls)
+        subprocess.call(quick_process, close_fds=False)
+        self.assertEqual([], self._warn_calls)
+        self.assertWarns(DeprecationWarning, subprocess.call, quick_process)
+        self.assertEqual(1, len(self._warn_calls))
+        self.assertIn('close_fds parameter was not specified',
+                      self._warn_calls[0][0])
+
+
 class ProcessTestCase(BaseTestCase):
 
     def test_call_seq(self):
@@ -1233,7 +1262,8 @@ def test_main():
                   ProcessTestCaseNoPoll,
                   HelperFunctionTests,
                   CommandsWithSpaces,
-                  ContextManagerTests)
+                  ContextManagerTests,
+                  DeprecationWarningTests)
 
     support.run_unittest(*unit_tests)
     support.reap_children()