From 483422f57e5d8c8bf8820fec29fc9b96bb15d4ef Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 5 Jul 2018 22:54:17 +0200 Subject: [PATCH] bpo-34044: subprocess.Popen copies startupinfo (GH-8090) subprocess.Popen now copies the startupinfo argument to leave it unchanged: it will modify the copy, so that the same STARTUPINFO object can be used multiple times. Add subprocess.STARTUPINFO.copy() method. --- Lib/subprocess.py | 17 ++++++++++++ Lib/test/test_subprocess.py | 27 +++++++++++++++++++ .../2018-07-04-17-14-26.bpo-34044.KWAu4y.rst | 3 +++ 3 files changed, 47 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2018-07-04-17-14-26.bpo-34044.KWAu4y.rst diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 93635ee61f7..e070011d980 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -135,6 +135,19 @@ if _mswindows: self.hStdError = hStdError self.wShowWindow = wShowWindow self.lpAttributeList = lpAttributeList or {"handle_list": []} + + def copy(self): + attr_list = self.lpAttributeList.copy() + if 'handle_list' in attr_list: + attr_list['handle_list'] = list(attr_list['handle_list']) + + return STARTUPINFO(dwFlags=self.dwFlags, + hStdInput=self.hStdInput, + hStdOutput=self.hStdOutput, + hStdError=self.hStdError, + wShowWindow=self.wShowWindow, + lpAttributeList=attr_list) + else: import _posixsubprocess import select @@ -1102,6 +1115,10 @@ class Popen(object): # Process startup details if startupinfo is None: startupinfo = STARTUPINFO() + else: + # bpo-34044: Copy STARTUPINFO since it is modified above, + # so the caller can reuse it multiple times. + startupinfo = startupinfo.copy() use_std_handles = -1 not in (p2cread, c2pwrite, errwrite) if use_std_handles: diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 4b089f525c1..73b57b21db2 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -2822,6 +2822,33 @@ class Win32ProcessTestCase(BaseTestCase): subprocess.call([sys.executable, "-c", "import sys; sys.exit(0)"], startupinfo=startupinfo) + def test_startupinfo_copy(self): + # bpo-34044: Popen must not modify input STARTUPINFO structure + startupinfo = subprocess.STARTUPINFO() + startupinfo.dwFlags = subprocess.STARTF_USESHOWWINDOW + startupinfo.wShowWindow = subprocess.SW_HIDE + + # Call Popen() twice with the same startupinfo object to make sure + # that it's not modified + for _ in range(2): + cmd = [sys.executable, "-c", "pass"] + with open(os.devnull, 'w') as null: + proc = subprocess.Popen(cmd, + stdout=null, + stderr=subprocess.STDOUT, + startupinfo=startupinfo) + with proc: + proc.communicate() + self.assertEqual(proc.returncode, 0) + + self.assertEqual(startupinfo.dwFlags, + subprocess.STARTF_USESHOWWINDOW) + self.assertIsNone(startupinfo.hStdInput) + self.assertIsNone(startupinfo.hStdOutput) + self.assertIsNone(startupinfo.hStdError) + self.assertEqual(startupinfo.wShowWindow, subprocess.SW_HIDE) + self.assertEqual(startupinfo.lpAttributeList, {"handle_list": []}) + def test_creationflags(self): # creationflags argument CREATE_NEW_CONSOLE = 16 diff --git a/Misc/NEWS.d/next/Library/2018-07-04-17-14-26.bpo-34044.KWAu4y.rst b/Misc/NEWS.d/next/Library/2018-07-04-17-14-26.bpo-34044.KWAu4y.rst new file mode 100644 index 00000000000..42a6ffbf84a --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-07-04-17-14-26.bpo-34044.KWAu4y.rst @@ -0,0 +1,3 @@ +``subprocess.Popen`` now copies the *startupinfo* argument to leave it +unchanged: it will modify the copy, so that the same ``STARTUPINFO`` object can +be used multiple times.