Enable get_number_open_files on more platforms, and use that to track down the...

Enable get_number_open_files on more platforms, and use that to track down the refcycle in FileObjectThread.

dev-requirements.txt

@@ -7,6 +7,7 @@ coverage>=4.0
 coveralls>=1.0
 cffi
 futures
+psutil
 # For viewing README.rst (restview --long-description),
 # CONTRIBUTING.rst, etc.
 # https://github.com/mgedmin/restview

src/gevent/fileobject.py

@@ -95,7 +95,7 @@ class FileObjectThread(FileObjectBase):
            the underlying object is closed as well.
         """
         closefd = close
-        self.threadpool = threadpool
+        self.threadpool = threadpool or get_hub().threadpool
         self.lock = lock
         if self.lock is True:
             self.lock = Semaphore()
@@ -112,8 +112,7 @@ class FileObjectThread(FileObjectBase):
                 fobj = os.fdopen(fobj)
             else:
                 fobj = os.fdopen(fobj, mode, bufsize)
-        if self.threadpool is None:
-            self.threadpool = get_hub().threadpool
+
         super(FileObjectThread, self).__init__(fobj, closefd)
 
     def _apply(self, func, args=None, kwargs=None):
@@ -160,6 +159,11 @@ class FileObjectThread(FileObjectBase):
     __next__ = next
 
     def _wrap_method(self, method):
+        # NOTE: This introduces a refcycle within self:
+        # self.__dict__ has methods that directly refer to self.
+        # Options to eliminate this are weakrefs, using __getattribute__ to
+        # fake a method descriptor, other? They all seem more costly than
+        # the refcycle.
         @functools.wraps(method)
         def thread_method(*args, **kwargs):
             if self._io is None:

src/greentest/greentest.py

@@ -633,11 +633,80 @@ def disabled_gc():
             gc.enable()
 
 
-def get_number_open_files():
-    if os.path.exists('/proc/'):
-        fd_directory = '/proc/%d/fd' % os.getpid()
-        return len(os.listdir(fd_directory))
+try:
+    import psutil
+except ImportError:
+    psutil = None
+    import re
+    # Linux/OS X/BSD platforms can implement this by calling out to lsof
+
+    def _run_lsof():
+        import tempfile
+        pid = os.getpid()
+        fd, tmpname = tempfile.mkstemp('get_open_files')
+        os.close(fd)
+        lsof_command = 'lsof -p %s > %s' % (pid, tmpname)
+        if os.system(lsof_command):
+            raise OSError("lsof failed")
+        with open(tmpname) as fobj:
+            data = fobj.read().strip()
+        os.remove(tmpname)
+        return data
+
+    def get_open_files(pipes=False):
+        data = _run_lsof()
+        results = {}
+        for line in data.split('\n'):
+            line = line.strip()
+            if not line or line.startswith("COMMAND"):
+                # Skip header and blank lines
+                continue
+            split = re.split(r'\s+', line)
+            command, pid, user, fd = split[:4]
+            # Pipes get an fd like "3" while normal files get an fd like "1u"
+            if fd[:-1].isdigit() or fd.isdigit():
+                if not pipes and not fd[-1].isdigit():
+                    continue
+                print(fd)
+                fd = int(fd[:-1]) if not fd[-1].isdigit() else int(fd)
+                if fd in results:
+                    params = (fd, line, split, results.get(fd), data)
+                    raise AssertionError('error when parsing lsof output: duplicate fd=%r\nline=%r\nsplit=%r\nprevious=%r\ndata:\n%s' % params)
+                results[fd] = line
+        if not results:
+            raise AssertionError('failed to parse lsof:\n%s' % (data, ))
+        results['data'] = data
+        return results
+
+    def get_number_open_files():
+        if os.path.exists('/proc/'):
+            # Linux only
+            fd_directory = '/proc/%d/fd' % os.getpid()
+            return len(os.listdir(fd_directory))
+        else:
+            try:
+                return len(get_open_files(pipes=True)) - 1
+            except (OSError, AssertionError):
+                return 0
 
+else:
+    # If psutil is available (it is cross-platform) use that.
+    # It is *much* faster than shelling out to lsof each time
+    # (Running 14 tests takes 3.964s with lsof and 0.046 with psutil)
+    # However, it still doesn't completely solve the issue on Windows: fds are reported
+    # as -1 there, so we can't fully check those.
+
+    def get_open_files():
+        results = dict()
+        process = psutil.Process()
+        results['data'] = process.open_files() + process.connections('all')
+        for x in results['data']:
+            results[x.fd] = x
+        return results
+
+    def get_number_open_files():
+        process = psutil.Process()
+        return process.num_fds()
 
 if PYPY:
 

src/greentest/test__makefile_ref.py

@@ -19,51 +19,11 @@ if PY3:
 fd_types = (int, long)
 WIN = sys.platform.startswith("win")
 
+from greentest import get_open_files
 try:
     import psutil
 except ImportError:
     psutil = None
-    # Linux/OS X/BSD platforms can implement this by calling out to lsof
-    tmpname = '/tmp/test__makefile_ref.lsof.%s' % pid
-    lsof_command = 'lsof -p %s > %s' % (pid, tmpname)
-
-    def get_open_files():
-        if os.system(lsof_command):
-            raise OSError('lsof failed')
-        with open(tmpname) as fobj:
-            data = fobj.read().strip()
-        results = {}
-        for line in data.split('\n'):
-            line = line.strip()
-            if not line:
-                continue
-            split = re.split(r'\s+', line)
-            command, pid, user, fd = split[:4]
-            if fd[:-1].isdigit() and not fd[-1].isdigit():
-                fd = int(fd[:-1])
-                if fd in results:
-                    params = (fd, line, split, results.get(fd), data)
-                    raise AssertionError('error when parsing lsof output: duplicate fd=%r\nline=%r\nsplit=%r\nprevious=%r\ndata:\n%s' % params)
-                results[fd] = line
-        if not results:
-            raise AssertionError('failed to parse lsof:\n%s' % (data, ))
-        results['data'] = data
-        return results
-else:
-    # If psutil is available (it is cross-platform) use that.
-    # It is *much* faster than shelling out to lsof each time
-    # (Running 14 tests takes 3.964s with lsof and 0.046 with psutil)
-    # However, it still doesn't completely solve the issue on Windows: fds are reported
-    # as -1 there, so we can't fully check those.
-    # XXX: Note: installing psutil on the travis linux vm caused failures.
-    process = psutil.Process()
-
-    def get_open_files():
-        results = dict()
-        results['data'] = process.open_files() + process.connections('all')
-        for x in results['data']:
-            results[x.fd] = x
-        return results
 
 
 class Test(unittest.TestCase):

src/greentest/test__subprocess.py

@@ -52,6 +52,8 @@ class Test(greentest.TestCase):
             raise AssertionError('Expected OSError: [Errno 2] No such file or directory')
 
     def test_leak(self):
+        from gevent.fileobject import FileObject, FileObjectThread
+
         num_before = greentest.get_number_open_files()
         p = subprocess.Popen([sys.executable, "-c", "print()"],
                              stdout=subprocess.PIPE)
@@ -60,9 +62,18 @@ class Test(greentest.TestCase):
         if PYPY:
             gc.collect()
             gc.collect()
+
         num_after = greentest.get_number_open_files()
+        if FileObject is FileObjectThread:
+            # There is (deliberately) a small refcycle within
+            # the instance itself
+            self.assertNotEqual(num_before, num_after)
+            gc.collect() # This produces a ResourceWarning
+            num_after = greentest.get_number_open_files()
+
         self.assertEqual(num_before, num_after)
 
+
     def test_communicate(self):
         p = subprocess.Popen([sys.executable, "-c",
                               'import sys,os;'