Fix process handles leak when uv_spawn() fails (#187)

Author: 1st1

.ci/requirements.txt

@@ -2,3 +2,4 @@ cython==0.28.3
 aiohttp
 tinys3
 twine
+psutil

requirements.dev.txt

@@ -1,2 +1,3 @@
 Cython==0.28.3
 Sphinx>=1.4.1
+psutil

tests/test_process.py

@@ -10,10 +10,15 @@
 import time
 import unittest
 
+import psutil
+
 from uvloop import _testbase as tb
 
 
 class _TestProcess:
+    def get_num_fds(self):
+        return psutil.Process(os.getpid()).num_fds()
+
     def test_process_env_1(self):
         async def test():
             cmd = 'echo $FOO$BAR'
@@ -330,6 +335,46 @@ async def test():
 
         self.loop.run_until_complete(test())
 
+    def test_subprocess_fd_leak_1(self):
+        async def main(n):
+            for i in range(n):
+                try:
+                    await asyncio.create_subprocess_exec(
+                        'nonexistant',
+                        loop=self.loop,
+                        stdout=subprocess.DEVNULL,
+                        stderr=subprocess.DEVNULL)
+                except FileNotFoundError:
+                    pass
+                await asyncio.sleep(0, loop=self.loop)
+
+        self.loop.run_until_complete(main(10))
+        num_fd_1 = self.get_num_fds()
+        self.loop.run_until_complete(main(10))
+        num_fd_2 = self.get_num_fds()
+
+        self.assertEqual(num_fd_1, num_fd_2)
+
+    def test_subprocess_fd_leak_2(self):
+        async def main(n):
+            for i in range(n):
+                try:
+                    p = await asyncio.create_subprocess_exec(
+                        'ls',
+                        loop=self.loop,
+                        stdout=subprocess.DEVNULL,
+                        stderr=subprocess.DEVNULL)
+                finally:
+                    await p.wait()
+                await asyncio.sleep(0, loop=self.loop)
+
+        self.loop.run_until_complete(main(10))
+        num_fd_1 = self.get_num_fds()
+        self.loop.run_until_complete(main(10))
+        num_fd_2 = self.get_num_fds()
+
+        self.assertEqual(num_fd_1, num_fd_2)
+
 
 class _AsyncioTests:
 

uvloop/handles/process.pxd

@@ -20,6 +20,8 @@ cdef class UVProcess(UVHandle):
         char *uv_opt_file
         bytes __cwd
 
+    cdef _close_process_handle(self)
+
     cdef _init(self, Loop loop, list args, dict env, cwd,
                start_new_session,
                _stdin, _stdout, _stderr, pass_fds,

uvloop/handles/process.pyx

@@ -11,6 +11,16 @@ cdef class UVProcess(UVHandle):
         self._preexec_fn = None
         self._restore_signals = True
 
+    cdef _close_process_handle(self):
+        # XXX: This is a workaround for a libuv bug:
+        # - https://github.com/libuv/libuv/issues/1933
+        # - https://github.com/libuv/libuv/pull/551
+        if self._handle is NULL:
+            return
+        self._handle.data = NULL
+        uv.uv_close(self._handle, __uv_close_process_handle_cb)
+        self._handle = NULL  # close callback will free() the memory
+
     cdef _init(self, Loop loop, list args, dict env,
                cwd, start_new_session,
                _stdin, _stdout, _stderr,  # std* can be defined as macros in C
@@ -79,16 +89,15 @@ cdef class UVProcess(UVHandle):
 
             if _PyImport_ReleaseLock() < 0:
                 # See CPython/posixmodule.c for details
+                self._close_process_handle()
                 if err < 0:
                     self._abort_init()
                 else:
                     self._close()
                 raise RuntimeError('not holding the import lock')
 
             if err < 0:
-                if UVLOOP_DEBUG and uv.uv_is_active(self._handle):
-                    raise RuntimeError(
-                        'active uv_process_t handle after failed uv_spawn')
+                self._close_process_handle()
                 self._abort_init()
                 raise convert_error(err)
 
@@ -754,3 +763,7 @@ cdef __socketpair():
     os_set_inheritable(fds[1], False)
 
     return fds[0], fds[1]
+
+
+cdef void __uv_close_process_handle_cb(uv.uv_handle_t* handle) with gil:
+    PyMem_RawFree(handle)