Skip to content

Commit

Permalink
src: restore stdio on program exit
Browse files Browse the repository at this point in the history
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: nodejs#14752
Fixes: nodejs#21020
Original-PR-URL: nodejs#20592
  • Loading branch information
bnoordhuis committed May 24, 2019
1 parent 3293bbf commit 256614a
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 8 deletions.
94 changes: 88 additions & 6 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@
#else
#include <pthread.h>
#include <sys/resource.h> // getrlimit, setrlimit
#include <termios.h> // tcgetattr, tcsetattr
#include <unistd.h> // STDIN_FILENO, STDERR_FILENO
#endif

Expand Down Expand Up @@ -191,7 +192,7 @@ void WaitForInspectorDisconnect(Environment* env) {

#ifdef __POSIX__
void SignalExit(int signo, siginfo_t* info, void* ucontext) {
uv_tty_reset_mode();
ResetStdio();
raise(signo);
}
#endif // __POSIX__
Expand Down Expand Up @@ -451,7 +452,7 @@ void TrapWebAssemblyOrContinue(int signo, siginfo_t* info, void* ucontext) {
if (prev != nullptr) {
prev(signo, info, ucontext);
} else {
uv_tty_reset_mode();
ResetStdio();
raise(signo);
}
}
Expand Down Expand Up @@ -481,6 +482,16 @@ void RegisterSignalHandler(int signal,

#endif // __POSIX__

#ifdef __POSIX__
static struct {
int flags;
bool isatty;
struct stat stat;
struct termios termios;
} stdio[1 + STDERR_FILENO];
#endif // __POSIX__


inline void PlatformInit() {
#ifdef __POSIX__
#if HAVE_INSPECTOR
Expand All @@ -491,16 +502,18 @@ inline void PlatformInit() {
#endif // HAVE_INSPECTOR

// Make sure file descriptors 0-2 are valid before we start logging anything.
for (int fd = STDIN_FILENO; fd <= STDERR_FILENO; fd += 1) {
struct stat ignored;
if (fstat(fd, &ignored) == 0)
for (auto& s : stdio) {
const int fd = &s - stdio;
if (fstat(fd, &s.stat) == 0)
continue;
// Anything but EBADF means something is seriously wrong. We don't
// have to special-case EINTR, fstat() is not interruptible.
if (errno != EBADF)
ABORT();
if (fd != open("/dev/null", O_RDWR))
ABORT();
if (fstat(fd, &s.stat) != 0)
ABORT();
}

#if HAVE_INSPECTOR
Expand All @@ -523,6 +536,27 @@ inline void PlatformInit() {
}
#endif // !NODE_SHARED_MODE

// Record the state of the stdio file descriptors so we can restore it
// on exit. Needs to happen before installing signal handlers because
// they make use of that information.
for (auto& s : stdio) {
const int fd = &s - stdio;
int err;

do
s.flags = fcntl(fd, F_GETFL);
while (s.flags == -1 && errno == EINTR); // NOLINT
CHECK_NE(s.flags, -1);

if (!isatty(fd)) continue;
s.isatty = true;

do
err = tcgetattr(fd, &s.termios);
while (err == -1 && errno == EINTR); // NOLINT
CHECK_EQ(err, 0);
}

RegisterSignalHandler(SIGINT, SignalExit, true);
RegisterSignalHandler(SIGTERM, SignalExit, true);

Expand Down Expand Up @@ -576,6 +610,54 @@ inline void PlatformInit() {
#endif // _WIN32
}


// Safe to call more than once and from signal handlers.
void ResetStdio() {
uv_tty_reset_mode();
#ifdef __POSIX__
for (auto& s : stdio) {
const int fd = &s - stdio;

struct stat tmp;
if (-1 == fstat(fd, &tmp)) {
CHECK_EQ(errno, EBADF); // Program closed file descriptor.
continue;
}

bool is_same_file =
(s.stat.st_dev == tmp.st_dev && s.stat.st_ino == tmp.st_ino);
if (!is_same_file) continue; // Program reopened file descriptor.

int flags;
do
flags = fcntl(fd, F_GETFL);
while (flags == -1 && errno == EINTR); // NOLINT
CHECK_NE(flags, -1);

// Restore the O_NONBLOCK flag if it changed.
if (O_NONBLOCK & (flags ^ s.flags)) {
flags &= ~O_NONBLOCK;
flags |= s.flags & O_NONBLOCK;

int err;
do
err = fcntl(fd, F_SETFL, flags);
while (err == -1 && errno == EINTR); // NOLINT
CHECK_NE(err, -1);
}

if (s.isatty) {
int err;
do
err = tcsetattr(fd, TCSANOW, &s.termios);
while (err == -1 && errno == EINTR); // NOLINT
CHECK_NE(err, -1);
}
}
#endif // __POSIX__
}


int ProcessGlobalArgs(std::vector<std::string>* args,
std::vector<std::string>* exec_args,
std::vector<std::string>* errors,
Expand Down Expand Up @@ -831,7 +913,7 @@ void Init(int* argc,
}

InitializationResult InitializeOncePerProcess(int argc, char** argv) {
atexit([] () { uv_tty_reset_mode(); });
atexit(ResetStdio);
PlatformInit();
per_process::node_start_time = uv_hrtime();

Expand Down
2 changes: 1 addition & 1 deletion src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ void AppendExceptionLine(Environment* env,
Mutex::ScopedLock lock(per_process::tty_mutex);
env->set_printed_error(true);

uv_tty_reset_mode();
ResetStdio();
PrintErrorString("\n%s", source.c_str());
return;
}
Expand Down
1 change: 1 addition & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ void PrintCaughtException(v8::Isolate* isolate,
const v8::TryCatch& try_catch);

void WaitForInspectorDisconnect(Environment* env);
void ResetStdio(); // Safe to call more than once and from signal handlers.
#ifdef __POSIX__
void SignalExit(int signal, siginfo_t* info, void* ucontext);
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/node_main_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ int NodeMainInstance::Run() {

env->set_can_call_into_js(false);
env->stop_sub_worker_contexts();
uv_tty_reset_mode();
ResetStdio();
env->RunCleanup();
RunAtExit(env.get());

Expand Down

0 comments on commit 256614a

Please sign in to comment.