From a908167e9835225c1fa129ce4b6fb5c903d0e314 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Tue, 22 Sep 2026 15:21:40 +0530 Subject: [PATCH] Fix the IPC lock used by custom shader compilation --- docs/changelog.rst | 5 +++++ kitty/utils.py | 13 +++++++++-- kitty_tests/datatypes.py | 48 ++++++++++++++++++++++++++-------------- 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index a3b98625c..9e629b3fe 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -206,6 +206,11 @@ Unreleased consistently after rearranging windows. Fix nested dividers not tracking mouse movement correctly, including when returning from a minimum size. +- Custom shaders: Fix starting a second kitty instance failing to build + shaders while another instance is building them, or after a previous + instance was killed while building them. Now instances wait for each other + to finish instead. + 0.49.0 [2026-09-21] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/kitty/utils.py b/kitty/utils.py index e2e6d300b..7ff766dd0 100644 --- a/kitty/utils.py +++ b/kitty/utils.py @@ -1239,11 +1239,20 @@ def unlock_file(f: IO[bytes] | IO[str]) -> None: @contextmanager def lock_with_file(path: str) -> Iterator[None]: - os.close(os.open(path, os.O_CREAT | os.O_WRONLY | os.O_EXCL | os.O_CLOEXEC)) + """Take an exclusive advisory lock on path, waiting for it to become available. + + The lock file is created if needed and left in place, since unlinking it + would allow a second process to create and lock a different file with the + same name while the lock is still held. The kernel drops the lock when the + file descriptor is closed, including on abnormal process exit, so stale lock + files are harmless. + """ + fd = os.open(path, os.O_CREAT | os.O_WRONLY | os.O_CLOEXEC, 0o600) try: + fcntl.flock(fd, fcntl.LOCK_EX) yield finally: - os.remove(path) + os.close(fd) def rmtree_best_effort(relpath: str, dir_fd: int) -> None: diff --git a/kitty_tests/datatypes.py b/kitty_tests/datatypes.py index abe82c397..b4d5e1ae6 100644 --- a/kitty_tests/datatypes.py +++ b/kitty_tests/datatypes.py @@ -758,34 +758,48 @@ class TestDataTypes(BaseTest): os.environ[k] = saved[k] def test_lock_with_file(self): + def is_locked_by_another_process(path): + # flock() locks are per file descriptor, so a non-blocking + # acquisition in this process would succeed even while the lock is + # held here, hence the check has to happen in a child process. + code = f'import fcntl, os;fd = os.open({path!r}, os.O_CREAT | os.O_WRONLY);fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)' + return subprocess.run([kitty_exe(), '+runpy', code], capture_output=True).returncode != 0 + with tempfile.TemporaryDirectory() as tdir: lock_path = os.path.join(tdir, 'test.lock') - # normal usage: file exists during context and is removed after + # normal usage: the lock is held only inside the context with lock_with_file(lock_path): self.assertTrue(os.path.exists(lock_path)) - self.assertFalse(os.path.exists(lock_path)) + self.assertTrue(is_locked_by_another_process(lock_path)) + self.assertFalse(is_locked_by_another_process(lock_path)) - # second acquisition of the same path succeeds after the first released it - with lock_with_file(lock_path): - self.assertTrue(os.path.exists(lock_path)) - self.assertFalse(os.path.exists(lock_path)) - - # file already exists: raises FileExistsError before yielding - open(lock_path, 'w').close() - with self.assertRaises(FileExistsError): - with lock_with_file(lock_path): - pass # should not be reached - # pre-existing file must not be deleted + # a pre-existing, unlocked lock file must not prevent acquisition self.assertTrue(os.path.exists(lock_path)) - os.remove(lock_path) + with lock_with_file(lock_path): + self.assertTrue(is_locked_by_another_process(lock_path)) + self.assertFalse(is_locked_by_another_process(lock_path)) - # lock file is removed even when the body raises + # the lock is released even when the body raises with self.assertRaises(RuntimeError): with lock_with_file(lock_path): - self.assertTrue(os.path.exists(lock_path)) raise RuntimeError('body error') - self.assertFalse(os.path.exists(lock_path)) + self.assertFalse(is_locked_by_another_process(lock_path)) + + # a waiting process acquires the lock once it is released + code = f'from kitty.utils import lock_with_file;lock_with_file({lock_path!r}).__enter__();print("acquired", flush=True)' + with lock_with_file(lock_path): + p = subprocess.Popen([kitty_exe(), '+runpy', code], stdout=subprocess.PIPE) + try: + # the child must block while we hold the lock + with self.assertRaises(subprocess.TimeoutExpired): + p.wait(timeout=1) + except Exception: + p.kill() + raise + self.assertEqual(p.stdout.readline(), b'acquired\n') + self.assertEqual(p.wait(timeout=30), 0) + p.stdout.close() def test_historybuf(self): lb = filled_line_buf()