From f21ca3aa14ba44129f4cd6b1aff1e8372aec6ace Mon Sep 17 00:00:00 2001 From: mr-mocap <16119203+mr-mocap@users.noreply.github.com> Date: Thu, 1 Dec 2022 16:56:35 -0500 Subject: [PATCH] Improve UNIX signal handling (#518) There was a dead lock caused by the reentrancy of the post method. Co-authored-by: ArthurSonzogni --- .../ftxui/component/screen_interactive.hpp | 7 +- src/ftxui/component/screen_interactive.cpp | 205 ++++++++++++------ 2 files changed, 140 insertions(+), 72 deletions(-) diff --git a/include/ftxui/component/screen_interactive.hpp b/include/ftxui/component/screen_interactive.hpp index 9aed718..f038c35 100644 --- a/include/ftxui/component/screen_interactive.hpp +++ b/include/ftxui/component/screen_interactive.hpp @@ -52,6 +52,8 @@ class ScreenInteractive : public Screen { Closure WithRestoredIO(Closure); private: + void ExitNow(); + void Install(); void Uninstall(); @@ -64,8 +66,9 @@ class ScreenInteractive : public Screen { void HandleTask(Component component, Task& task); void Draw(Component component); + void ResetCursorPosition(); - void SigStop(); + void Signal(int signal); ScreenInteractive* suspended_screen_ = nullptr; enum class Dimension { @@ -106,7 +109,7 @@ class ScreenInteractive : public Screen { public: class Private { public: - static void SigStop(ScreenInteractive& s) { return s.SigStop(); } + static void Signal(ScreenInteractive& s, int signal) { s.Signal(signal); } }; friend Private; }; diff --git a/src/ftxui/component/screen_interactive.cpp b/src/ftxui/component/screen_interactive.cpp index 90df36d..71762ce 100644 --- a/src/ftxui/component/screen_interactive.cpp +++ b/src/ftxui/component/screen_interactive.cpp @@ -148,7 +148,8 @@ void ftxui_on_resize(int columns, int rows) { } } -#else +#else // POSIX (Linux & Mac) + #include // for timeval int CheckStdinReady(int usec_timeout) { @@ -178,9 +179,74 @@ void EventListener(std::atomic* quit, Sender out) { } } } - #endif +std::stack on_exit_functions; // NOLINT +void OnExit() { + while (!on_exit_functions.empty()) { + on_exit_functions.top()(); + on_exit_functions.pop(); + } +} + +std::atomic g_signal_exit_count = 0; +#if !defined(_WIN32) +std::atomic g_signal_stop_count = 0; +std::atomic g_signal_resize_count = 0; +#endif + +// Async signal safe function +void RecordSignal(int signal) { + switch (signal) { + case SIGABRT: + case SIGFPE: + case SIGILL: + case SIGINT: + case SIGSEGV: + case SIGTERM: + g_signal_exit_count++; + break; + +#if !defined(_WIN32) + case SIGTSTP: + g_signal_stop_count++; + break; + + case SIGWINCH: + g_signal_resize_count++; + break; +#endif + + default: + break; + } +} + +void ExecuteSignalHandlers() { + int signal_exit_count = g_signal_exit_count.exchange(0); + while (signal_exit_count--) { + ScreenInteractive::Private::Signal(*g_active_screen, SIGABRT); + } + +#if !defined(_WIN32) + int signal_stop_count = g_signal_stop_count.exchange(0); + while (signal_stop_count--) { + ScreenInteractive::Private::Signal(*g_active_screen, SIGTSTP); + } + + int signal_resize_count = g_signal_resize_count.exchange(0); + while (signal_resize_count--) { + ScreenInteractive::Private::Signal(*g_active_screen, SIGWINCH); + } +#endif +} + +void InstallSignalHandler(int sig) { + auto old_signal_handler = std::signal(sig, RecordSignal); + on_exit_functions.push( + [=] { std::ignore = std::signal(sig, old_signal_handler); }); +}; + const std::string CSI = "\x1b["; // NOLINT // DEC: Digital Equipment Corporation @@ -230,31 +296,6 @@ std::string DeviceStatusReport(DSRMode ps) { return CSI + std::to_string(int(ps)) + "n"; } -using SignalHandler = void(int); -std::stack on_exit_functions; // NOLINT -void OnExit(int signal) { - (void)signal; - while (!on_exit_functions.empty()) { - on_exit_functions.top()(); - on_exit_functions.pop(); - } -} - -const auto install_signal_handler = [](int sig, SignalHandler handler) { - auto old_signal_handler = std::signal(sig, handler); - on_exit_functions.push( - [=] { std::ignore = std::signal(sig, old_signal_handler); }); -}; - -Closure g_on_resize = [] {}; // NOLINT -void OnResize(int /* signal */) { - g_on_resize(); -} - -void OnSigStop(int /*signal*/) { - ScreenInteractive::Private::SigStop(*g_active_screen); -} - class CapturedMouseImpl : public CapturedMouseInterface { public: explicit CapturedMouseImpl(std::function callback) @@ -378,10 +419,13 @@ void ScreenInteractive::PreMain() { // Suspend previously active screen: if (g_active_screen) { std::swap(suspended_screen_, g_active_screen); - std::cout << suspended_screen_->reset_cursor_position - << suspended_screen_->ResetPosition(/*clear=*/true); + // Reset cursor position to the top of the screen and clear the screen. + suspended_screen_->ResetCursorPosition(); + std::cout << suspended_screen_->ResetPosition(/*clear=*/true); suspended_screen_->dimx_ = 0; suspended_screen_->dimy_ = 0; + + // Reset dimensions to force drawing the screen again next time: suspended_screen_->Uninstall(); } @@ -393,20 +437,22 @@ void ScreenInteractive::PreMain() { } void ScreenInteractive::PostMain() { - g_active_screen->Uninstall(); - g_active_screen = nullptr; - // Put cursor position at the end of the drawing. - std::cout << reset_cursor_position; + ResetCursorPosition(); + + g_active_screen = nullptr; // Restore suspended screen. if (suspended_screen_) { + // Clear screen, and put the cursor at the beginning of the drawing. std::cout << ResetPosition(/*clear=*/true); dimx_ = 0; dimy_ = 0; + Uninstall(); std::swap(g_active_screen, suspended_screen_); g_active_screen->Install(); } else { + Uninstall(); // On final exit, keep the current drawing and reset cursor position one // line after it. std::cout << std::endl; @@ -430,6 +476,8 @@ ScreenInteractive* ScreenInteractive::Active() { } void ScreenInteractive::Install() { + frame_valid_ = false; + // After uninstalling the new configuration, flush it to the terminal to // ensure it is fully applied: on_exit_functions.push([] { Flush(); }); @@ -439,10 +487,10 @@ void ScreenInteractive::Install() { // Install signal handlers to restore the terminal state on exit. The default // signal handlers are restored on exit. for (int signal : {SIGTERM, SIGSEGV, SIGINT, SIGILL, SIGABRT, SIGFPE}) { - install_signal_handler(signal, OnExit); + InstallSignalHandler(signal); } - // Save the old terminal configuration and restore it on exit. +// Save the old terminal configuration and restore it on exit. #if defined(_WIN32) // Enable VT processing on stdout and stdin auto stdout_handle = GetStdHandle(STD_OUTPUT_HANDLE); @@ -474,6 +522,10 @@ void ScreenInteractive::Install() { SetConsoleMode(stdin_handle, in_mode); SetConsoleMode(stdout_handle, out_mode); #else + for (int signal : {SIGWINCH, SIGTSTP}) { + InstallSignalHandler(signal); + } + struct termios terminal; // NOLINT tcgetattr(STDIN_FILENO, &terminal); on_exit_functions.push([=] { tcsetattr(STDIN_FILENO, TCSANOW, &terminal); }); @@ -488,12 +540,6 @@ void ScreenInteractive::Install() { tcsetattr(STDIN_FILENO, TCSANOW, &terminal); - // Handle resize. - g_on_resize = [&] { task_sender_->Send(Event::Special({0})); }; - install_signal_handler(SIGWINCH, OnResize); - - // Handle SIGTSTP/SIGCONT. - install_signal_handler(SIGTSTP, OnSigStop); #endif auto enable = [&](const std::vector& parameters) { @@ -518,7 +564,7 @@ void ScreenInteractive::Install() { }); disable({ - //DECMode::kCursor, + // DECMode::kCursor, DECMode::kLineWrap, }); @@ -529,8 +575,8 @@ void ScreenInteractive::Install() { DECMode::kMouseSgrExtMode, }); - // After installing the new configuration, flush it to the terminal to ensure - // it is fully applied: + // After installing the new configuration, flush it to the terminal to + // ensure it is fully applied: Flush(); quit_ = false; @@ -542,28 +588,27 @@ void ScreenInteractive::Install() { } void ScreenInteractive::Uninstall() { - ExitLoopClosure()(); + ExitNow(); event_listener_.join(); animation_listener_.join(); - - OnExit(0); + OnExit(); } // NOLINTNEXTLINE void ScreenInteractive::RunOnceBlocking(Component component) { + ExecuteSignalHandlers(); Task task; if (task_receiver_->Receive(&task)) { HandleTask(component, task); } - RunOnce(component); } -// NOLINTNEXTLINE void ScreenInteractive::RunOnce(Component component) { Task task; while (task_receiver_->ReceiveNonBlocking(&task)) { HandleTask(component, task); + ExecuteSignalHandlers(); } Draw(std::move(component)); } @@ -650,7 +695,8 @@ void ScreenInteractive::Draw(Component component) { } bool resized = (dimx != dimx_) || (dimy != dimy_); - std::cout << reset_cursor_position << ResetPosition(/*clear=*/resized); + ResetCursorPosition(); + std::cout << ResetPosition(/*clear=*/resized); // Resize the screen if needed. if (resized) { @@ -710,7 +756,8 @@ void ScreenInteractive::Draw(Component component) { set_cursor_position += "\033[?25l"; } else { set_cursor_position += "\033[?25h"; - set_cursor_position += "\033[" + std::to_string(int(cursor_.shape)) + " q"; + set_cursor_position += + "\033[" + std::to_string(int(cursor_.shape)) + " q"; } } @@ -720,32 +767,50 @@ void ScreenInteractive::Draw(Component component) { frame_valid_ = true; } +void ScreenInteractive::ResetCursorPosition() { + std::cout << reset_cursor_position; + reset_cursor_position = ""; +} + Closure ScreenInteractive::ExitLoopClosure() { return [this] { Exit(); }; } void ScreenInteractive::Exit() { - Post([this] { - quit_ = true; - task_sender_.reset(); - }); + Post([this] { ExitNow(); }); } -void ScreenInteractive::SigStop() { -#if defined(_WIN32) - // Windows do no support SIGTSTP. -#else - Post([&] { - Uninstall(); - std::cout << reset_cursor_position; - reset_cursor_position = ""; - std::cout << ResetPosition(/*clear=*/true); - dimx_ = 0; - dimy_ = 0; - Flush(); - std::ignore = std::raise(SIGTSTP); - Install(); - }); +void ScreenInteractive::ExitNow() { + quit_ = true; + task_sender_.reset(); +} + +void ScreenInteractive::Signal(int signal) { + if (signal == SIGABRT) { + OnExit(); + return; + } + +// Windows do no support SIGTSTP / SIGWINCH +#if !defined(_WIN32) + if (signal == SIGTSTP) { + Post([&] { + ResetCursorPosition(); + std::cout << ResetPosition(/*clear*/ true); // Cursor to the beginning + Uninstall(); + dimx_ = 0; + dimy_ = 0; + Flush(); + std::ignore = std::raise(SIGTSTP); + Install(); + }); + return; + } + + if (signal == SIGWINCH) { + Post(Event::Special({0})); + return; + } #endif }