Skip to content

Commit 5872705

Browse files
bnoordhuisTrott
authored andcommitted
src: restore stdio on program exit
Record the state of the stdio file descriptors on start-up and restore them to that state on exit. This should prevent issues where node.js sometimes leaves stdio in raw or non-blocking mode. This is a reworked version of commit c2c9c0c from May 2018 that was reverted in commit 14dc17d from June 2018. The revert was a little light on details but I infer that the problem was caused by a missing call to `uv_tty_reset_mode()`. Apropos the NOLINT comments: cpplint doesn't understand do/while statements, it thinks they're while statements without a body. Fixes: #14752 Fixes: #21020 Original-PR-URL: #20592 PR-URL: #24260 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann (רפאל פלחי) <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
1 parent 0f90040 commit 5872705

File tree

4 files changed

+91
-8
lines changed

4 files changed

+91
-8
lines changed

src/node.cc

Lines changed: 88 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@
108108
#else
109109
#include <pthread.h>
110110
#include <sys/resource.h> // getrlimit, setrlimit
111+
#include <termios.h> // tcgetattr, tcsetattr
111112
#include <unistd.h> // STDIN_FILENO, STDERR_FILENO
112113
#endif
113114

@@ -194,7 +195,7 @@ void WaitForInspectorDisconnect(Environment* env) {
194195

195196
#ifdef __POSIX__
196197
void SignalExit(int signo, siginfo_t* info, void* ucontext) {
197-
uv_tty_reset_mode();
198+
ResetStdio();
198199
raise(signo);
199200
}
200201
#endif // __POSIX__
@@ -486,7 +487,7 @@ void TrapWebAssemblyOrContinue(int signo, siginfo_t* info, void* ucontext) {
486487
if (prev != nullptr) {
487488
prev(signo, info, ucontext);
488489
} else {
489-
uv_tty_reset_mode();
490+
ResetStdio();
490491
raise(signo);
491492
}
492493
}
@@ -516,6 +517,16 @@ void RegisterSignalHandler(int signal,
516517

517518
#endif // __POSIX__
518519

520+
#ifdef __POSIX__
521+
static struct {
522+
int flags;
523+
bool isatty;
524+
struct stat stat;
525+
struct termios termios;
526+
} stdio[1 + STDERR_FILENO];
527+
#endif // __POSIX__
528+
529+
519530
inline void PlatformInit() {
520531
#ifdef __POSIX__
521532
#if HAVE_INSPECTOR
@@ -526,16 +537,18 @@ inline void PlatformInit() {
526537
#endif // HAVE_INSPECTOR
527538

528539
// Make sure file descriptors 0-2 are valid before we start logging anything.
529-
for (int fd = STDIN_FILENO; fd <= STDERR_FILENO; fd += 1) {
530-
struct stat ignored;
531-
if (fstat(fd, &ignored) == 0)
540+
for (auto& s : stdio) {
541+
const int fd = &s - stdio;
542+
if (fstat(fd, &s.stat) == 0)
532543
continue;
533544
// Anything but EBADF means something is seriously wrong. We don't
534545
// have to special-case EINTR, fstat() is not interruptible.
535546
if (errno != EBADF)
536547
ABORT();
537548
if (fd != open("/dev/null", O_RDWR))
538549
ABORT();
550+
if (fstat(fd, &s.stat) != 0)
551+
ABORT();
539552
}
540553

541554
#if HAVE_INSPECTOR
@@ -558,6 +571,27 @@ inline void PlatformInit() {
558571
}
559572
#endif // !NODE_SHARED_MODE
560573

574+
// Record the state of the stdio file descriptors so we can restore it
575+
// on exit. Needs to happen before installing signal handlers because
576+
// they make use of that information.
577+
for (auto& s : stdio) {
578+
const int fd = &s - stdio;
579+
int err;
580+
581+
do
582+
s.flags = fcntl(fd, F_GETFL);
583+
while (s.flags == -1 && errno == EINTR); // NOLINT
584+
CHECK_NE(s.flags, -1);
585+
586+
if (!isatty(fd)) continue;
587+
s.isatty = true;
588+
589+
do
590+
err = tcgetattr(fd, &s.termios);
591+
while (err == -1 && errno == EINTR); // NOLINT
592+
CHECK_EQ(err, 0);
593+
}
594+
561595
RegisterSignalHandler(SIGINT, SignalExit, true);
562596
RegisterSignalHandler(SIGTERM, SignalExit, true);
563597

@@ -611,6 +645,54 @@ inline void PlatformInit() {
611645
#endif // _WIN32
612646
}
613647

648+
649+
// Safe to call more than once and from signal handlers.
650+
void ResetStdio() {
651+
uv_tty_reset_mode();
652+
#ifdef __POSIX__
653+
for (auto& s : stdio) {
654+
const int fd = &s - stdio;
655+
656+
struct stat tmp;
657+
if (-1 == fstat(fd, &tmp)) {
658+
CHECK_EQ(errno, EBADF); // Program closed file descriptor.
659+
continue;
660+
}
661+
662+
bool is_same_file =
663+
(s.stat.st_dev == tmp.st_dev && s.stat.st_ino == tmp.st_ino);
664+
if (!is_same_file) continue; // Program reopened file descriptor.
665+
666+
int flags;
667+
do
668+
flags = fcntl(fd, F_GETFL);
669+
while (flags == -1 && errno == EINTR); // NOLINT
670+
CHECK_NE(flags, -1);
671+
672+
// Restore the O_NONBLOCK flag if it changed.
673+
if (O_NONBLOCK & (flags ^ s.flags)) {
674+
flags &= ~O_NONBLOCK;
675+
flags |= s.flags & O_NONBLOCK;
676+
677+
int err;
678+
do
679+
err = fcntl(fd, F_SETFL, flags);
680+
while (err == -1 && errno == EINTR); // NOLINT
681+
CHECK_NE(err, -1);
682+
}
683+
684+
if (s.isatty) {
685+
int err;
686+
do
687+
err = tcsetattr(fd, TCSANOW, &s.termios);
688+
while (err == -1 && errno == EINTR); // NOLINT
689+
CHECK_NE(err, -1);
690+
}
691+
}
692+
#endif // __POSIX__
693+
}
694+
695+
614696
int ProcessGlobalArgs(std::vector<std::string>* args,
615697
std::vector<std::string>* exec_args,
616698
std::vector<std::string>* errors,
@@ -866,7 +948,7 @@ void Init(int* argc,
866948
}
867949

868950
InitializationResult InitializeOncePerProcess(int argc, char** argv) {
869-
atexit([] () { uv_tty_reset_mode(); });
951+
atexit(ResetStdio);
870952
PlatformInit();
871953
per_process::node_start_time = uv_hrtime();
872954

src/node_errors.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ void AppendExceptionLine(Environment* env,
230230
Mutex::ScopedLock lock(per_process::tty_mutex);
231231
env->set_printed_error(true);
232232

233-
uv_tty_reset_mode();
233+
ResetStdio();
234234
PrintErrorString("\n%s", source.c_str());
235235
return;
236236
}

src/node_internals.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ void PrintCaughtException(v8::Isolate* isolate,
9191
const v8::TryCatch& try_catch);
9292

9393
void WaitForInspectorDisconnect(Environment* env);
94+
void ResetStdio(); // Safe to call more than once and from signal handlers.
9495
#ifdef __POSIX__
9596
void SignalExit(int signal, siginfo_t* info, void* ucontext);
9697
#endif

src/node_main_instance.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ int NodeMainInstance::Run() {
145145

146146
env->set_can_call_into_js(false);
147147
env->stop_sub_worker_contexts();
148-
uv_tty_reset_mode();
148+
ResetStdio();
149149
env->RunCleanup();
150150
RunAtExit(env.get());
151151

0 commit comments

Comments
 (0)