Closes issue #103: file descriptor leak on EOF.issue-103-filedes-leak

Forcefully close the child pty descriptor when EOF is reached.

Without doing so, we have to wait until python's garbage collector
decides to call our __del__ method, which is not deterministic. So,
in such a retry loop as explained in issue #103, it is very possible
to reach "too many open files" on a subsequent fork() call.

This was done using TDD -- we first artificially lower our soft limit
of RLIMIT_NOFILE to be +10 than what we detect to be our limit by
quickly opening a temporary file and evaluating its current file
descriptor number.

Then, we loop N * 2 times of calling /bin/cat, expecting or catching
EOF in various forms to ensure we do not ever reach such "too many
open files" condition. Without the call to 'self.close()' in the EOF
handler, such condition is always reached at exactly the 10th loop.

Another condition must be handled in the expect_loop(), that, when
expecting [many, things, including, or not, EOF], EOF has previously
been caught, but one of the [many, things] are currently matching,
to ensure that those [many, things] are first returned. But once those
patterns have been exausted, check if self.after is set to EOF and to
re-raise the exception to allow the handler to also re-raise
it naturally. This is tested by
``test_donot_reach_max_ptys_expect_includes_EOF()``

3 files changed, 115 insertions, 4 deletions

diff --git a/doc/history.rst b/doc/history.rst
index 0da6c6e..6599ac0 100644
--- a/doc/history.rst
+++ b/doc/history.rst
@@ -4,6 +4,11 @@ History
 Releases
 --------
 
+Version 3.4
+```````````
+* Fixed leak of file descriptors when spawning very many child processes
+  (:ghissue:`103`).
+
 Version 3.3
 ```````````
 
diff --git a/pexpect/__init__.py b/pexpect/__init__.py
index 4a34f15..79516c5 100644
--- a/pexpect/__init__.py
+++ b/pexpect/__init__.py
@@ -385,7 +385,7 @@ class spawn(object):
 
             child = pexpect.spawn('some_command')
             child.logfile_read = sys.stdout
-        
+
         Remember to use spawnu instead of spawn for the above code if you are
         using Python 3.
 
@@ -1536,6 +1536,8 @@ class spawn(object):
                     self.match = searcher.match
                     self.match_index = index
                     return self.match_index
+                elif self.after == EOF:
+                    raise EOF("EOF: no matches after previously caught EOF.")
                 # No match at this point
                 if (timeout is not None) and (timeout < 0):
                     raise TIMEOUT('Timeout exceeded in expect_any().')
@@ -1552,6 +1554,15 @@ class spawn(object):
             self.before = incoming
             self.after = EOF
             index = searcher.eof_index
+            # before raising EOF, ensure the file descriptor(s) of the
+            # child have been closed.  Otherwise, it is up to python's
+            # garbage collector to call our __del__ method which does
+            # the same, but it is not deterministic.  If our object is
+            # never freed, we run the risk of a later fork() raising
+            # our own ExceptionPexpect exception with message,
+            #   "pty.fork() failed: out of pty devices."
+            self.close()
+
             if index >= 0:
                 self.match = EOF
                 self.match_index = index
diff --git a/tests/test_filedescriptor.py b/tests/test_filedescriptor.py
index d9164e1..e2b8565 100755
--- a/tests/test_filedescriptor.py
+++ b/tests/test_filedescriptor.py
@@ -20,13 +20,19 @@ PEXPECT LICENSE
 '''
 import pexpect
 from pexpect import fdpexpect
+import tempfile
+import resource
 import unittest
 from . import PexpectTestCase
 import os
 
-class ExpectTestCase(PexpectTestCase.PexpectTestCase):
+
+class TestFdexpect(PexpectTestCase.PexpectTestCase):
+    """
+    Test pexpect.fdexpect.fdspawn() wrapper.
+    """
+
     def setUp(self):
-        print(self.id())
         PexpectTestCase.PexpectTestCase.setUp(self)
 
     def test_fd (self):
@@ -66,7 +72,96 @@ class ExpectTestCase(PexpectTestCase.PexpectTestCase):
         assert not s.isalive()
         s.close()  # Smoketest - should be able to call this again
 
+
+class DescriptorLeakTestCase(PexpectTestCase.PexpectTestCase):
+    """
+    Test possibility of file descriptor leakage
+    """
+    #: ensure we have N number of open files available when limiting the
+    #: maximum number of open files for the current process. Prior to
+    #: forcefully closing the subprocess pipe on EOF, looping exactly
+    #: MAX_FILENO_BUMP is sufficient to reproduce the error.
+    MAX_FILENO_BUMP = 10
+
+    def setUp(self):
+        # assuming each file we open increments the file descriptor digit by
+        # one, create a temporary file, and assume we already have N files
+        # open, before setting a new limit of (N + 10).
+
+        # By artificially setting our soft limit, we forcefully cause a
+        # "too many open files" if there are any file descriptor leaks much
+        # earlier than otherwise.  On OSX, my default is 2,560 -- it would be
+        # a very long while until such limit is reached!
+        with tempfile.TemporaryFile() as fp:
+            MAX_NOFILES = fp.fileno() + self.MAX_FILENO_BUMP
+        self.save_limit_nofiles = resource.getrlimit(resource.RLIMIT_NOFILE)
+        _, hard = self.save_limit_nofiles
+        resource.setrlimit(resource.RLIMIT_NOFILE, (MAX_NOFILES, hard))
+        PexpectTestCase.PexpectTestCase.setUp(self)
+
+    def tearDown(self):
+        # restore (soft) max. num of open files limit
+        resource.setrlimit(resource.RLIMIT_NOFILE, self.save_limit_nofiles)
+
+    def test_donot_reach_max_ptys_caught_EOF(self):
+        """
+        Ensure spawning many subprocesses does not leak file descriptors.
+
+        Ensure that we do not depend on python's garbage collector to call
+        our __del__ method to subsequently "close" the object, and that
+        explicitly closing our object at EOF ensures we do not eventually
+        meet "pty.fork() failed: out of pty devices."
+        """
+        for _ in range(self.MAX_FILENO_BUMP * 2):
+            child = pexpect.spawn('/bin/cat', timeout=3)
+            child.sendeof()
+            try:
+                child.expect(['something', pexpect.TIMEOUT])
+            except pexpect.EOF:
+                pass
+
+    def test_donot_reach_max_ptys_expect_exact_EOF(self):
+        """
+        Ensure spawning many subprocesses does not leak file descriptors.
+
+        Behavior of catching EOF is different when it is .expect()ed directly.
+        """
+        for _ in range(self.MAX_FILENO_BUMP * 2):
+            child = pexpect.spawn('/bin/cat', timeout=3)
+            child.sendeof()
+            child.expect_exact(pexpect.EOF)
+
+    def test_donot_reach_max_ptys_expect_includes_EOF(self):
+        """
+        Ensure spawning many subprocesses does not leak file descriptors.
+
+        Behavior of catching EOF is different when it is expected by a list
+        of matches where EOF is included.
+        """
+        for _ in range(self.MAX_FILENO_BUMP * 2):
+            child = pexpect.spawn('/bin/cat', timeout=3)
+            child.sendline('alpha')
+            child.sendline('beta')
+            child.sendline('omega')
+            child.sendline('gamma')
+            child.sendline('delta')
+            child.sendeof()
+
+            # match some substrings, with EOF mixed in. We shouldn't
+            # yet reach EOF (even though we likely have, internally)
+            # in our return patterns.
+            assert child.expect([pexpect.EOF, 'alpha', 'beta']) == 1
+            assert child.expect(['omega', pexpect.EOF]) == 0
+            assert child.expect([pexpect.EOF, 'gamma']) == 1
+
+            # forcefully exhaust our search, reaching EOF
+            assert child.expect([pexpect.EOF]) == 0
+
+            # and you'll only get EOF thereon.
+            assert child.expect(['delta', pexpect.EOF]) == 1
+
+
 if __name__ == '__main__':
     unittest.main()
 
-suite = unittest.makeSuite(ExpectTestCase, 'test')
+suite = unittest.makeSuite(TestFdexpect, 'test')