Skip to content

Commit

Permalink
bpo-41818: Close file descriptors in test_openpty (#GH-24119)
Browse files Browse the repository at this point in the history
When stdin is a TTY, the test added in commit c13d899
is expected to fail. However, when it failed, it did not close
its file descriptors. This is flagged by the refleak tests (but
only when stdin is a TTY, which doesn't seem to be the case on CI).
  • Loading branch information
encukou authored Jan 19, 2021
1 parent bc450f9 commit 65cf1ad
Showing 1 changed file with 10 additions and 10 deletions.
20 changes: 10 additions & 10 deletions Lib/test/test_pty.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ def _set_term_winsz(fd, winsz):

# Marginal testing of pty suite. Cannot do extensive 'do or fail' testing
# because pty code is not too portable.
# XXX(nnorwitz): these tests leak fds when there is an error.
class PtyTest(unittest.TestCase):
def setUp(self):
old_alarm = signal.signal(signal.SIGALRM, self.handle_sig)
Expand Down Expand Up @@ -176,6 +175,12 @@ def test_openpty(self):
# " An optional feature could not be imported " ... ?
raise unittest.SkipTest("Pseudo-terminals (seemingly) not functional.")

# closing master_fd can raise a SIGHUP if the process is
# the session leader: we installed a SIGHUP signal handler
# to ignore this signal.
self.addCleanup(os.close, master_fd)
self.addCleanup(os.close, slave_fd)

self.assertTrue(os.isatty(slave_fd), "slave_fd is not a tty")

if mode:
Expand Down Expand Up @@ -218,15 +223,10 @@ def test_openpty(self):
s2 = _readline(master_fd)
self.assertEqual(b'For my pet fish, Eric.\n', normalize_output(s2))

os.close(slave_fd)
# closing master_fd can raise a SIGHUP if the process is
# the session leader: we installed a SIGHUP signal handler
# to ignore this signal.
os.close(master_fd)

def test_fork(self):
debug("calling pty.fork()")
pid, master_fd = pty.fork()
self.addCleanup(os.close, master_fd)
if pid == pty.CHILD:
# stdout should be connected to a tty.
if not os.isatty(1):
Expand Down Expand Up @@ -305,13 +305,14 @@ def test_fork(self):
##else:
## raise TestFailed("Read from master_fd did not raise exception")

os.close(master_fd)

def test_master_read(self):
# XXX(nnorwitz): this test leaks fds when there is an error.
debug("Calling pty.openpty()")
master_fd, slave_fd = pty.openpty()
debug(f"Got master_fd '{master_fd}', slave_fd '{slave_fd}'")

self.addCleanup(os.close, master_fd)

debug("Closing slave_fd")
os.close(slave_fd)

Expand All @@ -321,7 +322,6 @@ def test_master_read(self):
except OSError: # Linux
data = b""

os.close(master_fd)
self.assertEqual(data, b"")

class SmallPtyTests(unittest.TestCase):
Expand Down

0 comments on commit 65cf1ad

Please sign in to comment.