From 502a276820576e3cea68ad42fd6742c00d1ae490 Mon Sep 17 00:00:00 2001 From: Mikhail Lukashov Date: Thu, 4 Jun 2026 13:34:35 +0300 Subject: [PATCH] fix(tty,vt): prevent deadlocks and hangs in terminal backend Orphan detection uses prctl(PR_SET_PDEATHSIG, SIGUSR1) instead of getppid() polling, eliminating the PID reuse race and removing a syscall every 10ms from the suspend loop. Guard tcdrain, clipboard RPC, and Far2lInteract entry with _deadio to prevent indefinite blocking on dead file descriptors during shutdown. Far2lInteract uses a 10s TimedWait with _exiting/_deadio early-out to avoid blocking when the far2l peer disconnects. Fix SIGTSTP/SIGCONT handler swap in WinPortMainTTY cleanup. Two-phase lock in OnConsoleLog: release _inout_control_mutex before the blocking InterThreadCall to prevent main-thread starvation on mouse scroll during TUI app execution. Dispatch inter-thread calls and check pending ctrl events when NOOP_EVENT is received, preventing infinite spin when a delegate dispatch is pending. Drain kickass cancel byte inside LocalSocket::WaitForClient instead of in caller catch blocks to prevent rethrow loops for callers that retry after LocalSocketCancelled. --- WinPort/src/Backend/TTY/TTYBackend.cpp | 54 +++++++++++++++++++++++--- WinPort/src/Backend/TTY/TTYBackend.h | 5 ++- WinPort/src/Backend/TTY/TTYRevive.cpp | 4 -- WinPort/src/Backend/WinPortMain.cpp | 15 +++++++ far2l/src/console/keyboard.cpp | 7 ++++ far2l/src/vt/vtshell.cpp | 46 ++++++++++++++-------- far2l/src/vt/vtshell_ioreaders.h | 3 ++ utils/src/LocalSocket.cpp | 4 ++ 8 files changed, 110 insertions(+), 28 deletions(-) diff --git a/WinPort/src/Backend/TTY/TTYBackend.cpp b/WinPort/src/Backend/TTY/TTYBackend.cpp index e2e19987fb..bc4bb19c6d 100644 --- a/WinPort/src/Backend/TTY/TTYBackend.cpp +++ b/WinPort/src/Backend/TTY/TTYBackend.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #ifdef __linux__ @@ -32,6 +33,13 @@ static uint16_t g_far2l_term_width = 80, g_far2l_term_height = 25; static volatile long s_terminal_size_change_id = 0; static TTYBackend * g_vtb = nullptr; +static volatile sig_atomic_t s_parent_dead = 0; + +static void OnParentDead(int) +{ + s_parent_dead = 1; +} + long _iterm2_cmd_ts = 0; bool _iterm2_cmd_state = 0; @@ -86,6 +94,7 @@ TTYBackend::TTYBackend(const char *full_exe_path, int std_in, int std_out, bool _ext_clipboard(ext_clipboard), _restrict(restrict), _esc_expiration(esc_expiration), + _is_forktty_child(notify_pipe != -1), _notify_pipe(notify_pipe), _result(result), _largest_window_size_ready(false) @@ -306,7 +315,21 @@ void TTYBackend::ReaderThread() break; } usleep(10000); + if (_is_forktty_child && s_parent_dead) { + fprintf(stderr, "TTYBackend::ReaderThread: parent gone during suspend, exiting\n"); + _exiting = true; + WINPORT(GenerateConsoleCtrlEvent)(CTRL_CLOSE_EVENT, 0); + break; + } } else { + // If the parent session manager (ForkTTYChild wrapper) is gone, + // no one will ever connect for revival — exit gracefully. + if (_is_forktty_child && s_parent_dead) { + fprintf(stderr, "TTYBackend::ReaderThread: parent gone, exiting\n"); + _exiting = true; + WINPORT(GenerateConsoleCtrlEvent)(CTRL_CLOSE_EVENT, 0); + break; + } const std::string &info = StrWide2MB(g_winport_con_out->GetTitle()); int np = TTYReviveMe(_stdin, _stdout, _kickass[0], info); if (np != -1) { @@ -465,7 +488,9 @@ void TTYBackend::WriterThread() } tty_out.Flush(); - tcdrain(_stdout); + if (!_exiting && !_deadio) { + tcdrain(_stdout); + } if (ae.go_background) { gone_background = true; @@ -1020,7 +1045,7 @@ void TTYBackend::OSC52SetClipboard(const char *text) bool TTYBackend::Far2lInteract(StackSerializer &stk_ser, bool wait) { - if (_tty_caps.kind != TTYCaps::FAR2L || _exiting) + if (_tty_caps.kind != TTYCaps::FAR2L || _exiting || _deadio) return false; std::shared_ptr pfi = std::make_shared(); @@ -1037,10 +1062,24 @@ bool TTYBackend::Far2lInteract(StackSerializer &stk_ser, bool wait) if (!wait) return true; - pfi->evnt.Wait(); + static constexpr unsigned int FAR2L_INTERACT_TIMEOUT_MSEC = 10000; + static constexpr unsigned int FAR2L_INTERACT_POLL_MSEC = 500; + const auto deadline = std::chrono::steady_clock::now() + + std::chrono::milliseconds(FAR2L_INTERACT_TIMEOUT_MSEC); + bool completed = false; + while (!completed) { + completed = pfi->evnt.TimedWait(FAR2L_INTERACT_POLL_MSEC); + if (completed || _exiting || _deadio) + break; + if (std::chrono::steady_clock::now() >= deadline) { + fprintf(stderr, "TTYBackend::Far2lInteract: timeout\n"); + pfi->stk_ser.Swap(stk_ser); + return false; + } + } std::unique_lock lock_sent(_far2l_interacts_sent); - if (_exiting) + if (_exiting || _deadio) return false; pfi->stk_ser.Swap(stk_ser); @@ -1431,6 +1470,7 @@ static void OnSigHup(int signo) } } + void TTYBackend::OnSigHup() { // drop sudo privileges once pending sudo operation completes @@ -1656,15 +1696,17 @@ bool WinPortMainTTY(const char *full_exe_path, int std_in, int std_out, auto orig_tstp = signal(SIGTSTP, OnSigTstp); auto orig_cont = signal(SIGCONT, OnSigCont); auto orig_hup = signal(SIGHUP, (notify_pipe != -1) ? OnSigHup : SIG_DFL); // notify_pipe == -1 means --mortal specified + auto orig_usr1 = signal(SIGUSR1, (notify_pipe != -1) ? OnParentDead : SIG_DFL); *result = 0; // set OK status for case if app will go background *result = AppMain(argc, argv); signal(SIGHUP, orig_hup); - signal(SIGCONT, orig_tstp); - signal(SIGTSTP, orig_cont); + signal(SIGCONT, orig_cont); + signal(SIGTSTP, orig_tstp); signal(SIGWINCH, orig_winch); signal(SIGTERM, orig_term); + signal(SIGUSR1, orig_usr1); return true; } diff --git a/WinPort/src/Backend/TTY/TTYBackend.h b/WinPort/src/Backend/TTY/TTYBackend.h index 200940a32a..1350398c1c 100644 --- a/WinPort/src/Backend/TTY/TTYBackend.h +++ b/WinPort/src/Backend/TTY/TTYBackend.h @@ -45,6 +45,7 @@ class TTYBackend : IConsoleOutputBackend, ITTYInputSpecialSequenceHandler, IFar2 bool _ext_clipboard = false; TTYRestrict _restrict{}; unsigned int _esc_expiration = 0; + bool _is_forktty_child = false; int _notify_pipe = -1; int *_result = nullptr; int _kickass[2] = {-1, -1}; @@ -56,8 +57,8 @@ class TTYBackend : IConsoleOutputBackend, ITTYInputSpecialSequenceHandler, IFar2 long _terminal_size_change_id = 0; pthread_t _reader_trd = 0; - volatile bool _exiting = false; - volatile bool _deadio = false; + std::atomic _exiting{false}; + std::atomic _deadio{false}; static void *sReaderThread(void *p) { ((TTYBackend *)p)->ReaderThread(); return nullptr; } static void *sWriterThread(void *p) { ((TTYBackend *)p)->WriterThread(); return nullptr; } diff --git a/WinPort/src/Backend/TTY/TTYRevive.cpp b/WinPort/src/Backend/TTY/TTYRevive.cpp index 5dfccbb13f..a85aa09177 100644 --- a/WinPort/src/Backend/TTY/TTYRevive.cpp +++ b/WinPort/src/Backend/TTY/TTYRevive.cpp @@ -115,10 +115,6 @@ int TTYReviveMe(int std_in, int std_out, int kickass, const std::string &info) } catch (LocalSocketCancelled &e) { (void)e; fprintf(stderr, "TTYReviveMe: kickass signalled\n"); - char c; - if (read(kickass, &c, 1) < 0) { - perror("read kickass"); - } } catch (std::exception &e) { fprintf(stderr, "TTYReviveMe: %s\n", e.what()); diff --git a/WinPort/src/Backend/WinPortMain.cpp b/WinPort/src/Backend/WinPortMain.cpp index 834ecf4ccb..8d9747da78 100644 --- a/WinPort/src/Backend/WinPortMain.cpp +++ b/WinPort/src/Backend/WinPortMain.cpp @@ -12,9 +12,11 @@ # include # include # include +# include #elif defined(__FreeBSD__) || defined(__DragonFly__) # include # include +# include #endif #include @@ -356,6 +358,12 @@ static bool IsFar2lTerminal() return (env && strstr(env, "far2l") != NULL); } +static volatile sig_atomic_t s_pdeathsig_pending = 0; +static void OnParentDead(int) +{ + s_pdeathsig_pending = 1; +} + extern "C" int WinPortMain(const char *full_exe_path, int argc, char **argv, int(*AppMain)(int argc, char **argv)) { std::unique_ptr winport_con_out(new ConsoleOutput); @@ -560,6 +568,13 @@ extern "C" int WinPortMain(const char *full_exe_path, int argc, char **argv, int MakeFDCloexec(std_out); MakeFDCloexec(new_notify_pipe[1]); if (g_sigwinch_pid == 0) { + signal(SIGUSR1, OnParentDead); +#ifdef __linux__ + prctl(PR_SET_PDEATHSIG, SIGUSR1); +#elif defined(__FreeBSD__) || defined(__DragonFly__) + int sig = SIGUSR1; + procctl(P_PID, 0, PROC_PDEATHSIG_CTL, &sig); +#endif { setsid(); SudoAskpassImpl askass_impl; diff --git a/far2l/src/console/keyboard.cpp b/far2l/src/console/keyboard.cpp index b71847981b..0da3740194 100644 --- a/far2l/src/console/keyboard.cpp +++ b/far2l/src/console/keyboard.cpp @@ -34,6 +34,7 @@ THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "headers.hpp" #include +#include "InterThreadCall.hpp" #include "keyboard.hpp" #include "farqueue.hpp" #include "lang.hpp" @@ -708,6 +709,12 @@ static DWORD GetInputRecordInner(INPUT_RECORD *rec, bool ExcludeMacro, bool Proc } #endif + if (rec->EventType == NOOP_EVENT) { + Console.ReadInput(*rec); + DispatchInterThreadCalls(); + CheckForPendingCtrlHandleEvent(); + continue; + } break; } diff --git a/far2l/src/vt/vtshell.cpp b/far2l/src/vt/vtshell.cpp index f928dda3b5..b76844af3f 100644 --- a/far2l/src/vt/vtshell.cpp +++ b/far2l/src/vt/vtshell.cpp @@ -530,28 +530,42 @@ class VTShell : VTOutputReader::IProcessor, VTInputReader::IProcessor, IVTShell void OnConsoleLog(ConsoleLogKind kind)//NB: called not from main thread! { - std::unique_lock lock(_inout_control_mutex, std::try_to_lock); - if (!lock) { - fprintf(stderr, "VTShell::OnConsoleLog: SKIPPED\n"); - return; + // Two-phase lock discipline to avoid main-thread starvation: + // Phase 1: lock → stop output reader → unlock. + // VTA is suspended before the lock (VTAnsiSuspend RAII). + // Output reader stays stopped across the InterThreadCall. + // Phase 2: blocking InterThreadCall with NO mutex held — + // the main thread can freely call StartIOReaders/StopIOReaders. + // VTAnsiSuspend destructor fires at inner-scope exit: + // VTA is resumed BEFORE the reader is restarted. + // Phase 3: lock → restart output reader (only if we stopped it) → unlock. + bool did_stop = false; + { + VTAnsiSuspend vta_suspend(_vta); + if (!vta_suspend) + return; + { + std::lock_guard lock(_inout_control_mutex); + if (_output_reader.IsStarted()) { + _output_reader.Stop(); + did_stop = true; + } + } + DeliverPendingWindowInfo(); + InterThreadCall(std::bind(sShowConsoleLog, kind)); + } // VTA resumed here, before reader restart + // Restart the output reader so terminal output flows again. + if (did_stop) { + std::lock_guard lock(_inout_control_mutex); + if (!_output_reader.IsStarted()) { + _output_reader.Start(_fd_out); + } } - - //called in input thread context - //we're input, stop output and remember _vta state - - StopAndStart sas(_output_reader); - VTAnsiSuspend vta_suspend(_vta); - if (!vta_suspend) - return; - - DeliverPendingWindowInfo(); - InterThreadCall(std::bind(sShowConsoleLog, kind)); if (!_slavename.empty()) UpdateTerminalSize(_fd_out); if (_far2l_exts) _far2l_exts->OnTerminalResized(); } - void OnConsoleSwitch() { InterThreadLockAndWake ittlaw; diff --git a/far2l/src/vt/vtshell_ioreaders.h b/far2l/src/vt/vtshell_ioreaders.h index b38b13ede0..30153f9412 100644 --- a/far2l/src/vt/vtshell_ioreaders.h +++ b/far2l/src/vt/vtshell_ioreaders.h @@ -30,6 +30,8 @@ class WithThread volatile bool _started; bool Start(); + bool IsStarted() const { return _started; } + void Join(); virtual void OnJoin(); virtual void *ThreadProc() = 0; @@ -53,6 +55,7 @@ class VTOutputReader : protected WithThread void Start(int fd_out = -1); void Stop(); inline bool IsDeactivated() const { return _deactivated; } + inline bool IsStarted() const { return WithThread::IsStarted(); } void KickAss(); diff --git a/utils/src/LocalSocket.cpp b/utils/src/LocalSocket.cpp index bfefb6c955..6e30369234 100644 --- a/utils/src/LocalSocket.cpp +++ b/utils/src/LocalSocket.cpp @@ -227,6 +227,10 @@ void LocalSocketServer::WaitForClient(int fd_cancel, int tmout_msec) if (fd_cancel != -1) { if (FD_ISSET(fd_cancel, &fde) || FD_ISSET(fd_cancel, &fdr)) { + char c; + if (read(fd_cancel, &c, 1) > 0) { + // byte consumed — prevents immediate LocalSocketCancelled rethrow + } throw LocalSocketCancelled(); } }