Skip to content
Permalink
Browse files

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: nodejs/build#1820
Fixes: #28489

PR-URL: #28600
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information...
bnoordhuis authored and targos committed Jul 8, 2019
1 parent 209b353 commit 9f6600ac1cda89f9e28bf1a2dbc4a51d9e866c55
@@ -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.
@@ -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
@@ -1 +1,2 @@
Hello!

@@ -1 +1,2 @@
Hello!
<Buffer 48 65 6c 6c 6f 21 0a>
@@ -29,12 +29,14 @@

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 @@ def GetSource(self):
+ 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,
@@ -638,15 +638,10 @@ def RunProcess(context, timeout, args, **rest):
prev_error_mode = Win32SetErrorMode(error_mode)
Win32SetErrorMode(error_mode | prev_error_mode)

faketty = rest.pop('faketty', False)
pty_out = rest.pop('pty_out')

process = subprocess.Popen(
args = popen_args,
**rest
)
if faketty:
os.close(rest['stdout'])
if utils.IsWindows() and context.suppress_dialogs and prev_error_mode != SEM_INVALID_VALUE:
Win32SetErrorMode(prev_error_mode)
# Compute the end time - if the process crosses this limit we
@@ -658,28 +653,6 @@ def RunProcess(context, timeout, args, **rest):
# loop and keep track of whether or not it times out.
exit_code = None
sleep_time = INITIAL_SLEEP_TIME
output = ''
if faketty:
while True:
if time.time() >= end_time:
# Kill the process and wait for it to exit.
KillTimedOutProcess(context, process.pid)
exit_code = process.wait()
timed_out = True
break

# source: http://stackoverflow.com/a/12471855/1903116
# related: http://stackoverflow.com/q/11165521/1903116
try:
data = os.read(pty_out, 9999)
except OSError as e:
if e.errno != errno.EIO:
raise
break # EIO means EOF on some systems
else:
if not data: # EOF
break
output += data

while exit_code is None:
if (not end_time is None) and (time.time() >= end_time):
@@ -693,7 +666,7 @@ def RunProcess(context, timeout, args, **rest):
sleep_time = sleep_time * SLEEP_TIME_FACTOR
if sleep_time > MAX_SLEEP_TIME:
sleep_time = MAX_SLEEP_TIME
return (process, exit_code, timed_out, output)
return (process, exit_code, timed_out)


def PrintError(str):
@@ -715,31 +688,12 @@ def CheckedUnlink(name):
PrintError("os.unlink() " + str(e))
break

def Execute(args, context, timeout=None, env=None, faketty=False, disable_core_files=False, input=None):
def Execute(args, context, timeout=None, env=None, disable_core_files=False, stdin=None):
(fd_out, outname) = tempfile.mkstemp()
(fd_err, errname) = tempfile.mkstemp()

if env is None:
env = {}
if faketty:
import pty
(out_master, fd_out) = pty.openpty()
fd_in = fd_err = fd_out
pty_out = out_master

if input is not None:
# Before writing input data, disable echo so the input doesn't show
# up as part of the output.
import termios
attr = termios.tcgetattr(fd_in)
attr[3] = attr[3] & ~termios.ECHO
termios.tcsetattr(fd_in, termios.TCSADRAIN, attr)

os.write(pty_out, input)
os.write(pty_out, '\x04') # End-of-file marker (Ctrl+D)
else:
(fd_out, outname) = tempfile.mkstemp()
(fd_err, errname) = tempfile.mkstemp()
fd_in = 0
pty_out = None

env_copy = os.environ.copy()

# Remove NODE_PATH
@@ -758,28 +712,22 @@ def disableCoreFiles():
resource.setrlimit(resource.RLIMIT_CORE, (0,0))
preexec_fn = disableCoreFiles

(process, exit_code, timed_out, output) = RunProcess(
(process, exit_code, timed_out) = RunProcess(
context,
timeout,
args = args,
stdin = fd_in,
stdin = stdin,
stdout = fd_out,
stderr = fd_err,
env = env_copy,
faketty = faketty,
pty_out = pty_out,
preexec_fn = preexec_fn
)
if faketty:
os.close(out_master)
errors = ''
else:
os.close(fd_out)
os.close(fd_err)
output = open(outname).read()
errors = open(errname).read()
CheckedUnlink(outname)
CheckedUnlink(errname)
os.close(fd_out)
os.close(fd_err)
output = open(outname).read()
errors = open(errname).read()
CheckedUnlink(outname)
CheckedUnlink(errname)

return CommandOutput(exit_code, timed_out, output, errors)

0 comments on commit 9f6600a

Please sign in to comment.
You can’t perform that action at this time.