From 6604e0d015fbd7a3e5602a6f3831d786b4ed659d Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Tue, 30 Aug 2022 08:15:13 +0530 Subject: [PATCH] Fix regression in 0.26.0 that caused launching kitty without working STDIO handles to result in high CPU usage and prewarming failing Fixes #5444 --- docs/changelog.rst | 2 ++ kitty/launcher/main.c | 47 ++++++++++++++++++++++++++++++++++++++ kitty_tests/check_build.py | 16 +++++++++++++ 3 files changed, 65 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 2810d6c2d..6752e8009 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -40,6 +40,8 @@ Detailed list of changes - hyperlinked_grep kitten: Allow control which parts of rg output are hyperlinked (:pull:`5428`) +- Fix regression in 0.26.0 that caused launching kitty without working STDIO handles to result in high CPU usage and prewarming failing (:iss:`5444`) + 0.26.1 [2022-08-30] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/kitty/launcher/main.c b/kitty/launcher/main.c index 6ecc0ed22..0ec0661a3 100644 --- a/kitty/launcher/main.c +++ b/kitty/launcher/main.c @@ -17,6 +17,7 @@ #include #include #include +#include #ifndef KITTY_LIB_PATH #define KITTY_LIB_PATH "../.." @@ -268,9 +269,55 @@ read_exe_path(char *exe, size_t buf_sz) { } #endif // }}} +static bool +is_valid_fd(int fd) +{ + // This is copied from the python source code as we need the exact same semantics + // to prevent python from giving us None for sys.stdout and friends. +#if defined(F_GETFD) && ( \ + defined(__linux__) || \ + defined(__APPLE__) || \ + defined(__wasm__)) + return fcntl(fd, F_GETFD) >= 0; +#elif defined(__linux__) + int fd2 = dup(fd); + if (fd2 >= 0) { + close(fd2); + } + return (fd2 >= 0); +#else + struct stat st; + return (fstat(fd, &st) == 0); +#endif +} + +static bool +reopen_to_null(const char *mode, FILE *stream) { + errno = 0; + while (true) { + if (freopen("/dev/null", mode, stream) != NULL) return true; + if (errno == EINTR) continue; + perror("Failed to re-open STDIO handle to /dev/null"); + return false; + } +} + +static bool +ensure_working_stdio(void) { +#define C(which, mode) { \ + int fd = fileno(which); \ + if (fd < 0) { if (!reopen_to_null(mode, which)) return false; } \ + else if (!is_valid_fd(fd)) { \ + close(fd); if (!reopen_to_null(mode, which)) return false; \ + }} + C(stdin, "r") C(stdout, "w") C(stderr, "w") + return true; +#undef C +} int main(int argc, char *argv[], char* envp[]) { if (argc < 1 || !argv) { fprintf(stderr, "Invalid argc/argv\n"); return 1; } + if (!ensure_working_stdio()) return 1; char exe[PATH_MAX+1] = {0}; char exe_dir_buf[PATH_MAX+1] = {0}; FREE_AFTER_FUNCTION const char *lc_ctype = NULL; diff --git a/kitty_tests/check_build.py b/kitty_tests/check_build.py index 87a2edd16..4db7daa6c 100644 --- a/kitty_tests/check_build.py +++ b/kitty_tests/check_build.py @@ -107,6 +107,22 @@ def t(x, e): run_tests(partial(docs_url, local_docs_root=None), w, '/') self.ae(docs_url('#ref=issues-123'), 'https://github.com/kovidgoyal/kitty/issues/123') + def test_launcher_ensures_stdio(self): + from kitty.constants import kitty_exe + import subprocess + exe = kitty_exe() + cp = subprocess.run([exe, '+runpy', '''\ +import os, sys +if sys.stdin: + os.close(sys.stdin.fileno()) +if sys.stdout: + os.close(sys.stdout.fileno()) +if sys.stderr: + os.close(sys.stderr.fileno()) +os.execlp('kitty', 'kitty', '+runpy', 'import sys; raise SystemExit(1 if sys.stdout is None or sys.stdin is None or sys.stderr is None else 0)') +''']) + self.assertEqual(cp.returncode, 0) + def main() -> None: tests = unittest.defaultTestLoader.loadTestsFromTestCase(TestBuild)