FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: fix pty test hangs on aix · nodejs/node@7aca9cb · GitHub

/ node Public

Commit 7aca9cb

Browse files
bnoordhuis
authored and
BethGriggs
committed
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 Backport-PR-URL: #29599 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 588b761 commit 7aca9cb

6 files changed

Lines changed: 123 additions & 78 deletions

File tree

‎test/pseudo-tty/pseudo-tty.status‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,5 @@
11
prefix pseudo-tty
22

3-
[$system==aix]
4-
# being investigated under https://github.com/nodejs/node/issues/9728
5-
test-tty-wrap : FAIL, PASS
6-
# https://github.com/nodejs/build/issues/1820#issuecomment-505998851
7-
# https://github.com/nodejs/node/pull/28469
8-
console-dumb-tty: SKIP
9-
test-fatal-error: SKIP
10-
113
[$system==solaris]
124
# https://github.com/nodejs/node/pull/16225 - `ioctl(fd, TIOCGWINSZ)` seems
135
# to fail with EINVAL on SmartOS when `fd` is a pty from python's pty module.

‎test/pseudo-tty/pty_helper.py‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import errno
2+
import os
3+
import pty
4+
import select
5+
import signal
6+
import sys
7+
import termios
8+
9+
STDIN = 0
10+
STDOUT = 1
11+
STDERR = 2
12+
13+
14+
def pipe(sfd, dfd):
15+
try:
16+
data = os.read(sfd, 256)
17+
except OSError as e:
18+
if e.errno != errno.EIO:
19+
raise
20+
return True # EOF
21+
22+
if not data:
23+
return True # EOF
24+
25+
if dfd == STDOUT:
26+
# Work around platform quirks. Some platforms echo ^D as \x04
27+
# (AIX, BSDs) and some don't (Linux).
28+
filt = lambda c: ord(c) > 31 or c in '\t\n\r\f'
29+
data = filter(filt, data)
30+
31+
while data:
32+
try:
33+
n = os.write(dfd, data)
34+
except OSError as e:
35+
if e.errno != errno.EIO:
36+
raise
37+
return True # EOF
38+
data = data[n:]
39+
40+
41+
if __name__ == '__main__':
42+
argv = sys.argv[1:]
43+
44+
# Make select() interruptable by SIGCHLD.
45+
signal.signal(signal.SIGCHLD, lambda nr, _: None)
46+
47+
master_fd, slave_fd = pty.openpty()
48+
assert master_fd > STDIN
49+
50+
mode = termios.tcgetattr(slave_fd)
51+
# Don't translate \n to \r\n.
52+
mode[1] = mode[1] & ~termios.ONLCR # oflag
53+
# Disable ECHOCTL. It's a BSD-ism that echoes e.g. \x04 as ^D but it
54+
# doesn't work on platforms like AIX and Linux. I checked Linux's tty
55+
# driver and it's a no-op, the driver is just oblivious to the flag.
56+
mode[3] = mode[3] & ~termios.ECHOCTL # lflag
57+
termios.tcsetattr(slave_fd, termios.TCSANOW, mode)
58+
59+
pid = os.fork()
60+
if not pid:
61+
os.setsid()
62+
os.close(master_fd)
63+
64+
# Ensure the pty is a controlling tty.
65+
name = os.ttyname(slave_fd)
66+
fd = os.open(name, os.O_RDWR)
67+
os.dup2(fd, slave_fd)
68+
os.close(fd)
69+
70+
os.dup2(slave_fd, STDIN)
71+
os.dup2(slave_fd, STDOUT)
72+
os.dup2(slave_fd, STDERR)
73+
74+
if slave_fd > STDERR:
75+
os.close(slave_fd)
76+
77+
os.execve(argv[0], argv, os.environ)
78+
raise Exception('unreachable')
79+
80+
os.close(slave_fd)
81+
82+
fds = [STDIN, master_fd]
83+
while fds:
84+
try:
85+
rfds, _, _ = select.select(fds, [], [])
86+
except select.error as e:
87+
if e[0] != errno.EINTR:
88+
raise
89+
if pid == os.waitpid(pid, os.WNOHANG)[0]:
90+
break
91+
92+
if STDIN in rfds:
93+
if pipe(STDIN, master_fd):
94+
fds.remove(STDIN)
95+
96+
if master_fd in rfds:
97+
if pipe(master_fd, STDOUT):
98+
break

‎test/pseudo-tty/test-stdout-read.in‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
Hello!
2+


‎test/pseudo-tty/test-stdout-read.out‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1+
Hello!
12
<Buffer 48 65 6c 6c 6f 21 0a>

‎test/pseudo-tty/testcfg.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@
2727

2828
import test
2929
import os
30-
from os.path import join, exists, basename, isdir
30+
from os.path import join, exists, basename, dirname, isdir
3131
import re
32+
import sys
3233
import utils
3334

3435
FLAGS_PATTERN = re.compile(r"//\s+Flags:(.*)")
36+
PTY_HELPER = join(dirname(__file__), 'pty_helper.py')
3537

3638
class TTYTestCase(test.TestCase):
3739

@@ -105,17 +107,18 @@ def GetSource(self):
105107
+ open(self.expected).read())
106108

107109
def RunCommand(self, command, env):
108-
input = None
110+
fd = None
109111
if self.input is not None and exists(self.input):
110-
input = open(self.input).read()
112+
fd = os.open(self.input, os.O_RDONLY)
111113
full_command = self.context.processor(command)
114+
full_command = [sys.executable, PTY_HELPER] + full_command
112115
output = test.Execute(full_command,
113116
self.context,
114117
self.context.GetTimeout(self.mode),
115118
env,
116-
faketty=True,
117-
input=input)
118-
self.Cleanup()
119+
stdin=fd)
120+
if fd is not None:
121+
os.close(fd)
119122
return test.TestOutput(self,
120123
full_command,
121124
output,

‎tools/test.py‎

Lines changed: 14 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -640,15 +640,10 @@ def RunProcess(context, timeout, args, **rest):
640640
prev_error_mode = Win32SetErrorMode(error_mode);
641641
Win32SetErrorMode(error_mode | prev_error_mode);
642642

643-
faketty = rest.pop('faketty', False)
644-
pty_out = rest.pop('pty_out')
645-
646643
process = subprocess.Popen(
647644
args = popen_args,
648645
**rest
649646
)
650-
if faketty:
651-
os.close(rest['stdout'])
652647
if utils.IsWindows() and context.suppress_dialogs and prev_error_mode != SEM_INVALID_VALUE:
653648
Win32SetErrorMode(prev_error_mode)
654649
# Compute the end time - if the process crosses this limit we
@@ -660,28 +655,6 @@ def RunProcess(context, timeout, args, **rest):
660655
# loop and keep track of whether or not it times out.
661656
exit_code = None
662657
sleep_time = INITIAL_SLEEP_TIME
663-
output = ''
664-
if faketty:
665-
while True:
666-
if time.time() >= end_time:
667-
# Kill the process and wait for it to exit.
668-
KillTimedOutProcess(context, process.pid)
669-
exit_code = process.wait()
670-
timed_out = True
671-
break
672-
673-
# source: http://stackoverflow.com/a/12471855/1903116
674-
# related: http://stackoverflow.com/q/11165521/1903116
675-
try:
676-
data = os.read(pty_out, 9999)
677-
except OSError as e:
678-
if e.errno != errno.EIO:
679-
raise
680-
break # EIO means EOF on some systems
681-
else:
682-
if not data: # EOF
683-
break
684-
output += data
685658

686659
while exit_code is None:
687660
if (not end_time is None) and (time.time() >= end_time):
@@ -695,7 +668,7 @@ def RunProcess(context, timeout, args, **rest):
695668
sleep_time = sleep_time * SLEEP_TIME_FACTOR
696669
if sleep_time > MAX_SLEEP_TIME:
697670
sleep_time = MAX_SLEEP_TIME
698-
return (process, exit_code, timed_out, output)
671+
return (process, exit_code, timed_out)
699672

700673

701674
def PrintError(str):
@@ -717,29 +690,12 @@ def CheckedUnlink(name):
717690
PrintError("os.unlink() " + str(e))
718691
break
719692

720-
def Execute(args, context, timeout=None, env={}, faketty=False, disable_core_files=False, input=None):
721-
if faketty:
722-
import pty
723-
(out_master, fd_out) = pty.openpty()
724-
fd_in = fd_err = fd_out
725-
pty_out = out_master
726-
727-
if input is not None:
728-
# Before writing input data, disable echo so the input doesn't show
729-
# up as part of the output.
730-
import termios
731-
attr = termios.tcgetattr(fd_in)
732-
attr[3] = attr[3] & ~termios.ECHO
733-
termios.tcsetattr(fd_in, termios.TCSADRAIN, attr)
734-
735-
os.write(pty_out, input)
736-
os.write(pty_out, '\x04') # End-of-file marker (Ctrl+D)
737-
else:
738-
(fd_out, outname) = tempfile.mkstemp()
739-
(fd_err, errname) = tempfile.mkstemp()
740-
fd_in = 0
741-
pty_out = None
693+
def Execute(args, context, timeout=None, env=None, disable_core_files=False, stdin=None):
694+
(fd_out, outname) = tempfile.mkstemp()
695+
(fd_err, errname) = tempfile.mkstemp()
742696

697+
if env is None:
698+
env = {}
743699
env_copy = os.environ.copy()
744700

745701
# Remove NODE_PATH
@@ -758,28 +714,22 @@ def disableCoreFiles():
758714
resource.setrlimit(resource.RLIMIT_CORE, (0,0))
759715
preexec_fn = disableCoreFiles
760716

761-
(process, exit_code, timed_out, output) = RunProcess(
717+
(process, exit_code, timed_out) = RunProcess(
762718
context,
763719
timeout,
764720
args = args,
765-
stdin = fd_in,
721+
stdin = stdin,
766722
stdout = fd_out,
767723
stderr = fd_err,
768724
env = env_copy,
769-
faketty = faketty,
770-
pty_out = pty_out,
771725
preexec_fn = preexec_fn
772726
)
773-
if faketty:
774-
os.close(out_master)
775-
errors = ''
776-
else:
777-
os.close(fd_out)
778-
os.close(fd_err)
779-
output = file(outname).read()
780-
errors = file(errname).read()
781-
CheckedUnlink(outname)
782-
CheckedUnlink(errname)
727+
os.close(fd_out)
728+
os.close(fd_err)
729+
output = open(outname).read()
730+
errors = open(errname).read()
731+
CheckedUnlink(outname)
732+
CheckedUnlink(errname)
783733

784734
return CommandOutput(exit_code, timed_out, output, errors)
785735

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL