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