Mac view: checking the headset and publishing a tunnel happen under one short lock with retarget

Astra review: a switch landing between the check and the assignment could still
install the old headset's tunnel.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

tests/test_macview.py

@@ -10,6 +10,7 @@ Run: python3 -m unittest discover -s tests
 import base64
 import http.client
 import json
+import io
 import os
 import shutil
 import socket
@@ -59,6 +60,26 @@ class Helpers(unittest.TestCase):
         self.assertIsNone(mv.tunnel)
         self.assertEqual(mv.frame, "frame-2")
 
+    def test_a_switch_just_before_publishing_drops_the_old_headsets_tunnel(self):
+        mv = frame_macview.MacView(["ssh"], lambda *a, **k: "", "frame")
+        mv.port = 47000
+        ended = []
+
+        class Proc:
+            def poll(self): return None
+            def terminate(self): ended.append(self)
+            def wait(self): return 0
+            stderr = io.StringIO("")
+
+        def probe(port):
+            mv.retarget("frame-2", ["-o", "HostName=192.0.2.2"])  # the app switches right now
+            return True
+        with mock.patch.object(frame_macview.subprocess, "Popen", return_value=Proc()), \
+                mock.patch.object(frame_macview.time, "sleep"), mock.patch.object(mv, "_probe", probe):
+            self.assertFalse(mv._open_tunnel([], [47001]))
+        self.assertIsNone(mv.tunnel)
+        self.assertEqual(len(ended), 1)  # the tunnel to the old headset was closed
+
     @unittest.skipUnless(shutil.which("bash"), "needs bash")
     @unittest.skipIf(os.name == "nt", "Windows' bash.exe is WSL's launcher, and runners have no distribution")
     def test_launch_script_parses(self):

ui/frame_macview.py

@@ -121,6 +121,9 @@ class MacView:
         self.run = run
         self.frame = frame
         self.host_opts = []  # the headset in use and how to reach it (see retarget)
+        # Short, never held across ssh: retarget() and publishing a new tunnel check and
+        # change the headset together (self.lock is held while a tunnel is being opened).
+        self.route_lock = threading.Lock()
         self.track = track or (lambda proc: None)  # the server ends these on exit
         self.lock = threading.Lock()
         self.token = secrets.token_urlsafe(24)
@@ -242,27 +245,29 @@ class MacView:
 
     def retarget(self, alias, host_opts):
         """The server now reaches the headset as `alias` with `host_opts` (another address,
-        or another headset). No lock: this runs while the server routes, which a tunnel
-        being opened may be waiting on; each assignment is atomic."""
+        or another headset). Only the short route_lock, never self.lock: this runs while
+        the server routes, which a tunnel being opened may be waiting on."""
         # host_opts is "-o", "Name=value" pairs; the tunnel keeps its own connection,
         # not the shared master.
         opts = [x for flag, value in zip(host_opts[::2], host_opts[1::2])
                 if not value.startswith("ControlPath=") for x in (flag, value)]
-        moved = alias != self.frame
-        self.frame, self.host_opts = alias, opts
-        tunnel = self.tunnel
-        if moved and tunnel is not None:
-            # Another headset: its viewers can't be the old one's. The supervisor
-            # reopens a tunnel to the new one if anything is being shown.
-            self.tunnel, self.remote_port = None, None
-            if tunnel.poll() is None:
-                tunnel.terminate()
+        with self.route_lock:
+            moved = alias != self.frame
+            self.frame, self.host_opts = alias, opts
+            tunnel = self.tunnel
+            if moved and tunnel is not None:
+                # Another headset: its viewers can't be the old one's. The supervisor
+                # reopens a tunnel to the new one if anything is being shown.
+                self.tunnel, self.remote_port = None, None
+        if moved and tunnel is not None and tunnel.poll() is None:
+            tunnel.terminate()
 
     def _open_tunnel(self, via, ports):
         """Tries the ports on one route; True once the tunnel answers. With self.lock held."""
         last = ""
         for port in ports:
-            target = (self.frame, self.host_opts)  # retarget() may change these meanwhile
+            with self.route_lock:
+                target = (self.frame, self.host_opts)  # retarget() may change these meanwhile
             # `via` first: ssh keeps the first value of an option, so USB-C's HostName wins
             # while the headset's pinned identity (in host_opts) still checks it.
             proc = subprocess.Popen([*self.tunnel_ssh, "-o", "ControlPath=none", *via, *target[1],
@@ -281,11 +286,12 @@ class MacView:
                 if self._probe(port):
                     ok = True
                     break
-            if ok and target[0] != self.frame:
-                ok = False  # the app switched headset while this one connected: not its tunnel
-                self._last_tunnel_error = "switched headset"
             if ok:
-                self.tunnel, self.remote_port = proc, port
+                with self.route_lock:  # checked and published together, so a switch can't slip between
+                    ok = target[0] == self.frame  # else the app switched headset while this connected
+                    if ok:
+                        self.tunnel, self.remote_port = proc, port
+            if ok:
                 self.track(proc)
                 self._supervise()
                 return True