diff --git a/tests/test_macview.py b/tests/test_macview.py index b6a0ec1..e636927 100644 --- a/tests/test_macview.py +++ b/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): diff --git a/ui/frame_macview.py b/ui/frame_macview.py index 084e485..f6b346e 100644 --- a/ui/frame_macview.py +++ b/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