From 64c486e22c16faab08d2252781e36b6ac3a148f7 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Wed, 8 Jul 2026 16:33:02 +0200 Subject: [PATCH] libutil/libmain: never mask SIGWINCH we need to cache the current terminal size for progress bar reasons, but we don't want to catch SIGWINCH to update the current terminal size from a dedicated thread for repl reasons. a SIGWINCH handler function is much easier to square with these requirements than communication with another thread to have it change its signal mask, and since any races in handler code affect only progress bar output and only very rarely (if ever) we'd better chose the simplest approach. the progress bar could set a handler of its own for this purpose, but we would much rather replace it instead fixes #1246 Change-Id: I814d9aaf1b6fbb6a8cefc5af675a3aa372549dc8 --- lix/libcmd/repl-interacter.cc | 4 ++++ lix/libmain/shared.cc | 6 ------ lix/libutil/signals.cc | 8 ++++---- lix/libutil/terminal.cc | 25 ++++++++++++++++--------- lix/libutil/terminal.hh | 4 ++-- 5 files changed, 26 insertions(+), 21 deletions(-) diff --git a/lix/libcmd/repl-interacter.cc b/lix/libcmd/repl-interacter.cc index 9f11864a3..38364d49c 100644 --- a/lix/libcmd/repl-interacter.cc +++ b/lix/libcmd/repl-interacter.cc @@ -1,4 +1,5 @@ #include "libutil/fmt.hh" +#include "libutil/terminal.hh" #include "lix/libutil/error.hh" #include "lix/libutil/file-system.hh" #include "lix/libutil/logging.hh" @@ -65,6 +66,9 @@ bool ReadlineLikeInteracter::getLine(std::string & input, ReplPromptType promptT { auto s = rl->ask(promptForType(promptType)); + // rustyline temporarily sets a SIGWINCH handler + KJ_DEFER(invalidateWindowSize()); + return match_result( std::move(s), [&](rust::String ok) { diff --git a/lix/libmain/shared.cc b/lix/libmain/shared.cc index 6fdd27377..74fb300e6 100644 --- a/lix/libmain/shared.cc +++ b/lix/libmain/shared.cc @@ -176,12 +176,6 @@ void initNix() } #if __APPLE__ - /* HACK: on darwin, we need can’t use sigprocmask with SIGWINCH. - * Instead, add a dummy sigaction handler, and signalHandlerThread - * can handle the rest. */ - act.sa_handler = sigHandler; - if (sigaction(SIGWINCH, &act, 0)) throw SysError("handling SIGWINCH"); - /* Disable SA_RESTART for interrupts, so that system calls on this thread * error with EINTR like they do on Linux. * Most signals on BSD systems default to SA_RESTART on, but Nix diff --git a/lix/libutil/signals.cc b/lix/libutil/signals.cc index 0227b5f81..ad8b2f3a0 100644 --- a/lix/libutil/signals.cc +++ b/lix/libutil/signals.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -118,8 +119,6 @@ static void signalHandlerThread(sigset_t set) } } else if (signal == SIGTERM || signal == SIGHUP) { triggerInterrupt(); - } else if (signal == SIGWINCH) { - updateWindowSize(); } } } @@ -164,7 +163,7 @@ void saveSignalMask() { void startSignalHandlerThread() { - updateWindowSize(); + invalidateWindowSize(); saveSignalMask(); sigset_t set; @@ -173,10 +172,11 @@ void startSignalHandlerThread() sigaddset(&set, SIGTERM); sigaddset(&set, SIGHUP); sigaddset(&set, SIGPIPE); - sigaddset(&set, SIGWINCH); if (pthread_sigmask(SIG_BLOCK, &set, nullptr)) throw SysError("blocking signals"); + signal(SIGWINCH, [](int) { invalidateWindowSize(); }); + std::thread(signalHandlerThread, set).detach(); } diff --git a/lix/libutil/terminal.cc b/lix/libutil/terminal.cc index 162dc3a23..104bdb9c5 100644 --- a/lix/libutil/terminal.cc +++ b/lix/libutil/terminal.cc @@ -183,22 +183,29 @@ std::string filterANSIEscapes(std::string_view s, bool filterAll, unsigned int w return t; } -static Sync> windowSize{{0, 0}}; +static volatile sig_atomic_t windowSizeInvalid = 0; +static Sync> windowSize; -void updateWindowSize() +void invalidateWindowSize() { - struct winsize ws; - if (ioctl(2, TIOCGWINSZ, &ws) == 0 || ioctl(1, TIOCGWINSZ, &ws) == 0) { - auto windowSize_(windowSize.lock()); - windowSize_->first = ws.ws_row; - windowSize_->second = ws.ws_col; - } + windowSizeInvalid = 1; } std::pair getWindowSize() { - return *windowSize.lock(); + auto windowSize_(windowSize.lock()); + // this is racy, but given that window sizes change very rarely we'll just accept it + if (windowSizeInvalid) { + windowSizeInvalid = 0; + struct winsize ws; + if (ioctl(2, TIOCGWINSZ, &ws) == 0 || ioctl(1, TIOCGWINSZ, &ws) == 0) { + windowSize_->first = ws.ws_row; + windowSize_->second = ws.ws_col; + } + } + + return *windowSize_; } std::string makeHyperlink(std::string_view linkText, std::string_view target) diff --git a/lix/libutil/terminal.hh b/lix/libutil/terminal.hh index 867941e5f..97178aec1 100644 --- a/lix/libutil/terminal.hh +++ b/lix/libutil/terminal.hh @@ -57,10 +57,10 @@ std::string filterANSIEscapes(std::string_view s, bool eatTabs = true); /** - * Recalculate the window size, updating a global variable. Used in the + * Invalidate the window size, updating a global variable. Used in the * `SIGWINCH` signal handler. */ -void updateWindowSize(); +void invalidateWindowSize(); /** * @return the number of rows and columns of the terminal.