An interpreted, interactive, object-oriented programming language
CentOS Sources
2017-08-01 71084d584ff953f5463757ec6536406320560b4d
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
 
# HG changeset patch
# User Antoine Pitrou <solipsis@pitrou.net>
# Date 1377898693 -7200
# Node ID 43749cb6bdbd0fdab70f76cd171c3c02a3f600dd
# Parent  ba54011aa295004ad87438211fe3bb1568dd69ab
Issue #18851: Avoid a double close of subprocess pipes when the child process fails starting.
 
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -698,12 +698,12 @@ class Popen(object):
 
         (p2cread, p2cwrite,
          c2pread, c2pwrite,
-         errread, errwrite) = self._get_handles(stdin, stdout, stderr)
+         errread, errwrite), to_close = self._get_handles(stdin, stdout, stderr)
 
         try:
             self._execute_child(args, executable, preexec_fn, close_fds,
                                 cwd, env, universal_newlines,
-                                startupinfo, creationflags, shell,
+                                startupinfo, creationflags, shell, to_close,
                                 p2cread, p2cwrite,
                                 c2pread, c2pwrite,
                                 errread, errwrite)
@@ -711,18 +711,12 @@ class Popen(object):
             # Preserve original exception in case os.close raises.
             exc_type, exc_value, exc_trace = sys.exc_info()
 
-            to_close = []
-            # Only close the pipes we created.
-            if stdin == PIPE:
-                to_close.extend((p2cread, p2cwrite))
-            if stdout == PIPE:
-                to_close.extend((c2pread, c2pwrite))
-            if stderr == PIPE:
-                to_close.extend((errread, errwrite))
-
             for fd in to_close:
                 try:
-                    os.close(fd)
+                    if mswindows:
+                        fd.Close()
+                    else:
+                        os.close(fd)
                 except EnvironmentError:
                     pass
 
@@ -816,8 +810,9 @@ class Popen(object):
             """Construct and return tuple with IO objects:
             p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite
             """
+            to_close = set()
             if stdin is None and stdout is None and stderr is None:
-                return (None, None, None, None, None, None)
+                return (None, None, None, None, None, None), to_close
 
             p2cread, p2cwrite = None, None
             c2pread, c2pwrite = None, None
@@ -835,6 +830,10 @@ class Popen(object):
                 # Assuming file-like object
                 p2cread = msvcrt.get_osfhandle(stdin.fileno())
             p2cread = self._make_inheritable(p2cread)
+            # We just duplicated the handle, it has to be closed at the end
+            to_close.add(p2cread)
+            if stdin == PIPE:
+                to_close.add(p2cwrite)
 
             if stdout is None:
                 c2pwrite = _subprocess.GetStdHandle(_subprocess.STD_OUTPUT_HANDLE)
@@ -848,6 +847,10 @@ class Popen(object):
                 # Assuming file-like object
                 c2pwrite = msvcrt.get_osfhandle(stdout.fileno())
             c2pwrite = self._make_inheritable(c2pwrite)
+            # We just duplicated the handle, it has to be closed at the end
+            to_close.add(c2pwrite)
+            if stdout == PIPE:
+                to_close.add(c2pread)
 
             if stderr is None:
                 errwrite = _subprocess.GetStdHandle(_subprocess.STD_ERROR_HANDLE)
@@ -863,10 +866,14 @@ class Popen(object):
                 # Assuming file-like object
                 errwrite = msvcrt.get_osfhandle(stderr.fileno())
             errwrite = self._make_inheritable(errwrite)
+            # We just duplicated the handle, it has to be closed at the end
+            to_close.add(errwrite)
+            if stderr == PIPE:
+                to_close.add(errread)
 
             return (p2cread, p2cwrite,
                     c2pread, c2pwrite,
-                    errread, errwrite)
+                    errread, errwrite), to_close
 
 
         def _make_inheritable(self, handle):
@@ -895,7 +902,7 @@ class Popen(object):
 
         def _execute_child(self, args, executable, preexec_fn, close_fds,
                            cwd, env, universal_newlines,
-                           startupinfo, creationflags, shell,
+                           startupinfo, creationflags, shell, to_close,
                            p2cread, p2cwrite,
                            c2pread, c2pwrite,
                            errread, errwrite):
@@ -934,6 +941,10 @@ class Popen(object):
                     # kill children.
                     creationflags |= _subprocess.CREATE_NEW_CONSOLE
 
+            def _close_in_parent(fd):
+                fd.Close()
+                to_close.remove(fd)
+
             # Start the process
             try:
                 hp, ht, pid, tid = _subprocess.CreateProcess(executable, args,
@@ -958,11 +969,11 @@ class Popen(object):
                 # pipe will not close when the child process exits and the
                 # ReadFile will hang.
                 if p2cread is not None:
-                    p2cread.Close()
+                    _close_in_parent(p2cread)
                 if c2pwrite is not None:
-                    c2pwrite.Close()
+                    _close_in_parent(c2pwrite)
                 if errwrite is not None:
-                    errwrite.Close()
+                    _close_in_parent(errwrite)
 
             # Retain the process handle, but close the thread handle
             self._child_created = True
@@ -1088,6 +1099,7 @@ class Popen(object):
             """Construct and return tuple with IO objects:
             p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite
             """
+            to_close = set()
             p2cread, p2cwrite = None, None
             c2pread, c2pwrite = None, None
             errread, errwrite = None, None
@@ -1096,6 +1108,7 @@ class Popen(object):
                 pass
             elif stdin == PIPE:
                 p2cread, p2cwrite = self.pipe_cloexec()
+                to_close.update((p2cread, p2cwrite))
             elif isinstance(stdin, int):
                 p2cread = stdin
             else:
@@ -1106,6 +1119,7 @@ class Popen(object):
                 pass
             elif stdout == PIPE:
                 c2pread, c2pwrite = self.pipe_cloexec()
+                to_close.update((c2pread, c2pwrite))
             elif isinstance(stdout, int):
                 c2pwrite = stdout
             else:
@@ -1116,6 +1130,7 @@ class Popen(object):
                 pass
             elif stderr == PIPE:
                 errread, errwrite = self.pipe_cloexec()
+                to_close.update((errread, errwrite))
             elif stderr == STDOUT:
                 errwrite = c2pwrite
             elif isinstance(stderr, int):
@@ -1126,7 +1141,7 @@ class Popen(object):
 
             return (p2cread, p2cwrite,
                     c2pread, c2pwrite,
-                    errread, errwrite)
+                    errread, errwrite), to_close
 
 
         def _set_cloexec_flag(self, fd, cloexec=True):
@@ -1170,7 +1185,7 @@ class Popen(object):
 
         def _execute_child(self, args, executable, preexec_fn, close_fds,
                            cwd, env, universal_newlines,
-                           startupinfo, creationflags, shell,
+                           startupinfo, creationflags, shell, to_close,
                            p2cread, p2cwrite,
                            c2pread, c2pwrite,
                            errread, errwrite):
@@ -1189,6 +1204,10 @@ class Popen(object):
             if executable is None:
                 executable = args[0]
 
+            def _close_in_parent(fd):
+                os.close(fd)
+                to_close.remove(fd)
+
             # For transferring possible exec failure from child to parent
             # The first char specifies the exception type: 0 means
             # OSError, 1 means some other error.
@@ -1283,17 +1302,17 @@ class Popen(object):
                     # be sure the FD is closed no matter what
                     os.close(errpipe_write)
 
-                if p2cread is not None and p2cwrite is not None:
-                    os.close(p2cread)
-                if c2pwrite is not None and c2pread is not None:
-                    os.close(c2pwrite)
-                if errwrite is not None and errread is not None:
-                    os.close(errwrite)
-
                 # Wait for exec to fail or succeed; possibly raising exception
                 # Exception limited to 1M
                 data = _eintr_retry_call(os.read, errpipe_read, 1048576)
             finally:
+                if p2cread is not None and p2cwrite is not None:
+                    _close_in_parent(p2cread)
+                if c2pwrite is not None and c2pread is not None:
+                    _close_in_parent(c2pwrite)
+                if errwrite is not None and errread is not None:
+                    _close_in_parent(errwrite)
+
                 # be sure the FD is closed no matter what
                 os.close(errpipe_read)
 
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -14,6 +14,10 @@ try:
     import resource
 except ImportError:
     resource = None
+try:
+    import threading
+except ImportError:
+    threading = None
 
 mswindows = (sys.platform == "win32")
 
@@ -629,6 +633,36 @@ class ProcessTestCase(BaseTestCase):
             if c.exception.errno not in (errno.ENOENT, errno.EACCES):
                 raise c.exception
 
+    @unittest.skipIf(threading is None, "threading required")
+    def test_double_close_on_error(self):
+        # Issue #18851
+        fds = []
+        def open_fds():
+            for i in range(20):
+                fds.extend(os.pipe())
+                time.sleep(0.001)
+        t = threading.Thread(target=open_fds)
+        t.start()
+        try:
+            with self.assertRaises(EnvironmentError):
+                subprocess.Popen(['nonexisting_i_hope'],
+                                 stdin=subprocess.PIPE,
+                                 stdout=subprocess.PIPE,
+                                 stderr=subprocess.PIPE)
+        finally:
+            t.join()
+            exc = None
+            for fd in fds:
+                # If a double close occurred, some of those fds will
+                # already have been closed by mistake, and os.close()
+                # here will raise.
+                try:
+                    os.close(fd)
+                except OSError as e:
+                    exc = e
+            if exc is not None:
+                raise exc
+
     def test_handles_closed_on_exception(self):
         # If CreateProcess exits with an error, ensure the
         # duplicate output handles are released
@@ -783,7 +817,7 @@ class POSIXProcessTestCase(BaseTestCase)
 
         def _execute_child(
                 self, args, executable, preexec_fn, close_fds, cwd, env,
-                universal_newlines, startupinfo, creationflags, shell,
+                universal_newlines, startupinfo, creationflags, shell, to_close,
                 p2cread, p2cwrite,
                 c2pread, c2pwrite,
                 errread, errwrite):
@@ -791,7 +825,7 @@ class POSIXProcessTestCase(BaseTestCase)
                 subprocess.Popen._execute_child(
                         self, args, executable, preexec_fn, close_fds,
                         cwd, env, universal_newlines,
-                        startupinfo, creationflags, shell,
+                        startupinfo, creationflags, shell, to_close,
                         p2cread, p2cwrite,
                         c2pread, c2pwrite,
                         errread, errwrite)