From 1d05f57fbf731790611de7eb30cfc9d1729e19b3 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Mon, 28 Sep 2026 21:14:27 +1000 Subject: [PATCH] Mac in the headset: a Show waits for a viewer cleanup already in progress Review round 11: the cleanup's check and pkill now hold a lock that Show takes to count itself in, so a new viewer can't start between them. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_macview.py | 18 ++++++++++++++++++ ui/frame_macview.py | 20 ++++++++++++-------- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/tests/test_macview.py b/tests/test_macview.py index 54f8641..20bf152 100644 --- a/tests/test_macview.py +++ b/tests/test_macview.py @@ -18,6 +18,7 @@ import subprocess import sys import tempfile import time +import threading import unittest from unittest import mock from pathlib import Path @@ -101,6 +102,23 @@ class Helpers(unittest.TestCase): mv.launching = 1 # a Show replacing its own stream is still launching mv._end_viewer_browser(mv.shows) self.assertEqual(len(calls), 1) + # A Show waits while the cleanup checks and runs pkill. + mv.launching = 0 + mv._show = lambda *a: "shown" + started = threading.Event() + + def slow_pkill(remote, **kw): + started.set() + time.sleep(0.3) + calls.append("after " + remote) + mv.run = slow_pkill + with mock.patch.object(frame_macview.time, "sleep"): + t = threading.Thread(target=mv._end_viewer_browser, args=(mv.shows,)) + t.start() + started.wait(2) + self.assertEqual(mv.show("window:5"), "shown") # blocked until the pkill finished + self.assertTrue(calls[-1].startswith("after ")) + t.join() class WS: diff --git a/ui/frame_macview.py b/ui/frame_macview.py index d96dd29..00b4d04 100644 --- a/ui/frame_macview.py +++ b/ui/frame_macview.py @@ -130,6 +130,9 @@ class MacView: self.closing = False self.shows = 0 # counts Show presses, so a late cleanup can't close a new viewer self.launching = 0 # Shows in progress (one may be replacing its own stream) + # Held by a Show while it counts itself in, and by the cleanup for its + # check and its pkill together, so a Show can't start in between. + self.viewer_lock = threading.Lock() self.shown = set() # sources with a viewer out there, connected or retrying self.browser_flags = list(BROWSER_FLAGS) @@ -281,12 +284,12 @@ class MacView: # ---- viewers on the Frame ---- def show(self, src, quality="balanced", width=None, height=None): - with self.lock: + with self.viewer_lock: self.launching += 1 try: return self._show(src, quality, width, height) finally: - with self.lock: + with self.viewer_lock: self.launching -= 1 def _show(self, src, quality, width, height): @@ -347,12 +350,13 @@ class MacView: 2026-09-28), so once nothing is shown, end it. It runs with a profile of its own, so nothing else is touched.""" time.sleep(2) # the viewers close their windows first - if self.shown or self.shows != shows or self.launching: - return - try: - self.run("pkill -f '[f]rame-control/mac-view|[d]ata/frame-mac-view' || true", timeout=10) - except Exception: - pass + with self.viewer_lock: + if self.shown or self.shows != shows or self.launching: + return + try: + self.run("pkill -f '[f]rame-control/mac-view|[d]ata/frame-mac-view' || true", timeout=10) + except Exception: + pass def state(self): reason = self.unavailable()