test: fix pty test hangs on aix

Some pty tests persistently hung on the AIX CI buildbots. Fix that by
adding a helper script that properly sets up the pty before spawning
the script under test.

On investigation I discovered that the test runner hung when it tried
to close the slave pty's file descriptor, probably due to a bug in
AIX's pty implementation. I could reproduce it with a short C program.
The test runner also leaked file descriptors to the child process.

I couldn't convince python's `subprocess.Popen()` to do what I wanted
it to do so I opted to move the logic to a helper script that can do
fork/setsid/etc. without having to worry about stomping on state in
tools/test.py.

In the process I also uncovered some bugs in the pty module of the
python distro that ships with macOS 10.14, leading me to reimplement
a sizable chunk of the functionality of that module.

And last but not least, of course there are differences between ptys
on different platforms and the helper script has to paper over that.
Of course.

Really, this commit took me longer to put together than I care to admit.

Caveat emptor: this commit takes the hacky ^D feeding to the slave out
of tools/test.py and puts it in the *.in input files. You can also feed
other control characters to tests, like ^C or ^Z, simply by inserting
them into the corresponding input file. I think that's nice.

Fixes: https://github.com/nodejs/build/issues/1820
Fixes: https://github.com/nodejs/node/issues/28489

PR-URL: https://github.com/nodejs/node/pull/28600
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

5 files changed, 109 insertions, 40 deletions

diff --git a/test/pseudo-tty/pseudo-tty.status b/test/pseudo-tty/pseudo-tty.status
index 9d5a2a0d12..0a302b2d56 100644
--- a/test/pseudo-tty/pseudo-tty.status
+++ b/test/pseudo-tty/pseudo-tty.status
@@ -1,40 +1,5 @@
 prefix pseudo-tty
 
-[$system==aix]
-# https://github.com/nodejs/build/issues/1820#issuecomment-505998851
-# https://github.com/nodejs/node/pull/28469
-# https://github.com/nodejs/node/pull/28541
-console_colors: SKIP
-console-dumb-tty: SKIP
-no_dropped_stdio: SKIP
-no_interleaved_stdio: SKIP
-readline-dumb-tty: SKIP
-ref_keeps_node_running: SKIP
-repl-dumb-tty: SKIP
-stdin-setrawmode: SKIP
-test-assert-colors: SKIP
-test-assert-no-color: SKIP
-test-assert-position-indicator: SKIP
-test-async-wrap-getasyncid-tty: SKIP
-test-fatal-error: SKIP
-test-handle-wrap-isrefed-tty: SKIP
-test-readable-tty-keepalive: SKIP
-test-set-raw-mode-reset-process-exit: SKIP
-test-set-raw-mode-reset-signal: SKIP
-test-set-raw-mode-reset: SKIP
-test-stderr-stdout-handle-sigwinch: SKIP
-test-stdin-write: SKIP
-test-stdout-read: SKIP
-test-tty-color-support: SKIP
-test-tty-isatty: SKIP
-test-tty-stdin-call-end: SKIP
-test-tty-stdin-end: SKIP
-test-tty-stdout-end: SKIP
-test-tty-stdout-resize: SKIP
-test-tty-stream-constructors: SKIP
-test-tty-window-size: SKIP
-test-tty-wrap: SKIP
-
 [$system==solaris]
 # https://github.com/nodejs/node/pull/16225 - `ioctl(fd, TIOCGWINSZ)` seems
 # to fail with EINVAL on SmartOS when `fd` is a pty from python's pty module.
diff --git a/test/pseudo-tty/pty_helper.py b/test/pseudo-tty/pty_helper.py
new file mode 100644
index 0000000000..d0a4b945d9
--- /dev/null
+++ b/test/pseudo-tty/pty_helper.py
@@ -0,0 +1,98 @@
+import errno
+import os
+import pty
+import select
+import signal
+import sys
+import termios
+
+STDIN = 0
+STDOUT = 1
+STDERR = 2
+
+
+def pipe(sfd, dfd):
+  try:
+    data = os.read(sfd, 256)
+  except OSError as e:
+    if e.errno != errno.EIO:
+      raise
+    return True  # EOF
+
+  if not data:
+    return True  # EOF
+
+  if dfd == STDOUT:
+    # Work around platform quirks. Some platforms echo ^D as \x04
+    # (AIX, BSDs) and some don't (Linux).
+    filt = lambda c: ord(c) > 31 or c in '\t\n\r\f'
+    data = filter(filt, data)
+
+  while data:
+    try:
+      n = os.write(dfd, data)
+    except OSError as e:
+      if e.errno != errno.EIO:
+        raise
+      return True  # EOF
+    data = data[n:]
+
+
+if __name__ == '__main__':
+  argv = sys.argv[1:]
+
+  # Make select() interruptable by SIGCHLD.
+  signal.signal(signal.SIGCHLD, lambda nr, _: None)
+
+  master_fd, slave_fd = pty.openpty()
+  assert master_fd > STDIN
+
+  mode = termios.tcgetattr(slave_fd)
+  # Don't translate \n to \r\n.
+  mode[1] = mode[1] & ~termios.ONLCR  # oflag
+  # Disable ECHOCTL. It's a BSD-ism that echoes e.g. \x04 as ^D but it
+  # doesn't work on platforms like AIX and Linux. I checked Linux's tty
+  # driver and it's a no-op, the driver is just oblivious to the flag.
+  mode[3] = mode[3] & ~termios.ECHOCTL  # lflag
+  termios.tcsetattr(slave_fd, termios.TCSANOW, mode)
+
+  pid = os.fork()
+  if not pid:
+    os.setsid()
+    os.close(master_fd)
+
+    # Ensure the pty is a controlling tty.
+    name = os.ttyname(slave_fd)
+    fd = os.open(name, os.O_RDWR)
+    os.dup2(fd, slave_fd)
+    os.close(fd)
+
+    os.dup2(slave_fd, STDIN)
+    os.dup2(slave_fd, STDOUT)
+    os.dup2(slave_fd, STDERR)
+
+    if slave_fd > STDERR:
+      os.close(slave_fd)
+
+    os.execve(argv[0], argv, os.environ)
+    raise Exception('unreachable')
+
+  os.close(slave_fd)
+
+  fds = [STDIN, master_fd]
+  while fds:
+    try:
+      rfds, _, _ = select.select(fds, [], [])
+    except select.error as e:
+      if e[0] != errno.EINTR:
+        raise
+      if pid == os.waitpid(pid, os.WNOHANG)[0]:
+        break
+
+    if STDIN in rfds:
+      if pipe(STDIN, master_fd):
+        fds.remove(STDIN)
+
+    if master_fd in rfds:
+      if pipe(master_fd, STDOUT):
+        break
diff --git a/test/pseudo-tty/test-stdout-read.in b/test/pseudo-tty/test-stdout-read.in
index 10ddd6d257..d7bda826cf 100644
--- a/test/pseudo-tty/test-stdout-read.in
+++ b/test/pseudo-tty/test-stdout-read.in
@@ -1 +1,2 @@
 Hello!
+
\ No newline at end of file
diff --git a/test/pseudo-tty/test-stdout-read.out b/test/pseudo-tty/test-stdout-read.out
index 3b7fda223d..e92928d17b 100644
--- a/test/pseudo-tty/test-stdout-read.out
+++ b/test/pseudo-tty/test-stdout-read.out
@@ -1 +1,2 @@
+Hello!
 <Buffer 48 65 6c 6c 6f 21 0a>
diff --git a/test/pseudo-tty/testcfg.py b/test/pseudo-tty/testcfg.py
index 96f596d886..8f09bef2e9 100644
--- a/test/pseudo-tty/testcfg.py
+++ b/test/pseudo-tty/testcfg.py
@@ -29,12 +29,14 @@ from __future__ import print_function
 
 import test
 import os
-from os.path import join, exists, basename, isdir
+from os.path import join, exists, basename, dirname, isdir
 import re
+import sys
 import utils
 from functools import reduce
 
 FLAGS_PATTERN = re.compile(r"//\s+Flags:(.*)")
+PTY_HELPER = join(dirname(__file__), 'pty_helper.py')
 
 class TTYTestCase(test.TestCase):
 
@@ -108,16 +110,18 @@ class TTYTestCase(test.TestCase):
           + open(self.expected).read())
 
   def RunCommand(self, command, env):
-    input_arg = None
+    fd = None
     if self.input is not None and exists(self.input):
-      input_arg = open(self.input).read()
+      fd = os.open(self.input, os.O_RDONLY)
     full_command = self.context.processor(command)
+    full_command = [sys.executable, PTY_HELPER] + full_command
     output = test.Execute(full_command,
                      self.context,
                      self.context.GetTimeout(self.mode),
                      env,
-                     faketty=True,
-                     input=input_arg)
+                     stdin=fd)
+    if fd is not None:
+      os.close(fd)
     return test.TestOutput(self,
                       full_command,
                       output,