From b6689d93bfb5400b3cdabcd8b2f059d0e1f3df2f Mon Sep 17 00:00:00 2001 From: Storm Dragon Date: Fri, 8 May 2026 00:48:11 -0400 Subject: [PATCH] Use single shared remote command lock --- src/fenrirscreenreader/core/remoteManager.py | 44 +++++++------------- tests/integration/test_remote_control.py | 24 ++++++++++- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/src/fenrirscreenreader/core/remoteManager.py b/src/fenrirscreenreader/core/remoteManager.py index d872cfe4..37fa8c30 100644 --- a/src/fenrirscreenreader/core/remoteManager.py +++ b/src/fenrirscreenreader/core/remoteManager.py @@ -39,7 +39,7 @@ from fenrirscreenreader.core.i18n import _ REMOTE_COMMAND_LOCK_TIMEOUT_SEC = 2.0 -REMOTE_COMMAND_LOCK_PREFIX = "fenrirscreenreader-remote-" +REMOTE_COMMAND_LOCK_FILE = "fenrirscreenreader-remote-command.lock" class RemoteManager: @@ -282,28 +282,10 @@ class RemoteManager: return def _get_remote_command_lock_path(self, event_data): - event_hash = hashlib.sha256(event_data.encode("utf-8")).hexdigest() - return os.path.join( - tempfile.gettempdir(), - f"{REMOTE_COMMAND_LOCK_PREFIX}{event_hash}.lock", - ) + return os.path.join(tempfile.gettempdir(), REMOTE_COMMAND_LOCK_FILE) - def _cleanup_stale_remote_command_locks(self, now): - try: - lock_files = os.listdir(tempfile.gettempdir()) - except OSError: - return - - for lock_file in lock_files: - if not lock_file.startswith(REMOTE_COMMAND_LOCK_PREFIX): - continue - lock_path = os.path.join(tempfile.gettempdir(), lock_file) - try: - lock_age = now - os.stat(lock_path).st_mtime - if lock_age > REMOTE_COMMAND_LOCK_TIMEOUT_SEC: - os.unlink(lock_path) - except OSError: - pass + def _get_remote_command_hash(self, event_data): + return hashlib.sha256(event_data.encode("utf-8")).hexdigest() def _read_remote_command_lock(self, lock_file): lock_file.seek(0) @@ -312,12 +294,13 @@ class RemoteManager: lock_pid = int(lock_parts[0]) if lock_parts else 0 except ValueError: lock_pid = 0 - return lock_pid + lock_hash = lock_parts[1] if len(lock_parts) > 1 else "" + return lock_pid, lock_hash - def _write_remote_command_lock(self, lock_file, now): + def _write_remote_command_lock(self, lock_file, event_hash, now): lock_file.seek(0) lock_file.truncate() - lock_file.write(f"{os.getpid()} {now}\n") + lock_file.write(f"{os.getpid()} {event_hash} {now}\n") lock_file.flush() os.fsync(lock_file.fileno()) @@ -335,8 +318,8 @@ class RemoteManager: def _claim_remote_command(self, event_data): lock_path = self._get_remote_command_lock_path(event_data) + event_hash = self._get_remote_command_hash(event_data) now = time.time() - self._cleanup_stale_remote_command_locks(now) lock_file = self._open_remote_command_lock(lock_path) if lock_file is None: return False @@ -344,20 +327,21 @@ class RemoteManager: with lock_file: try: fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX) - lock_pid = self._read_remote_command_lock(lock_file) + lock_pid, lock_hash = self._read_remote_command_lock(lock_file) lock_stat = os.fstat(lock_file.fileno()) if lock_pid == os.getpid(): + self._write_remote_command_lock(lock_file, event_hash, now) return True - if not lock_pid: - self._write_remote_command_lock(lock_file, now) + if not lock_pid or lock_hash != event_hash: + self._write_remote_command_lock(lock_file, event_hash, now) return True if now - lock_stat.st_mtime < REMOTE_COMMAND_LOCK_TIMEOUT_SEC: return False - self._write_remote_command_lock(lock_file, now) + self._write_remote_command_lock(lock_file, event_hash, now) return True finally: fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN) diff --git a/tests/integration/test_remote_control.py b/tests/integration/test_remote_control.py index 9b875f58..a66af6c5 100644 --- a/tests/integration/test_remote_control.py +++ b/tests/integration/test_remote_control.py @@ -250,14 +250,16 @@ class TestRemoteDataFormat: """Test untargeted duplicate remote commands only run in one process.""" self.manager.initialize(mock_environment) lock_path = tmp_path / "remote-command.lock" - lock_path.write_text("999999 1\n") + event_data = "command say duplicated" + event_hash = self.manager._get_remote_command_hash(event_data) + lock_path.write_text(f"999999 {event_hash} {time.time()}\n") with patch.object( self.manager, "_get_remote_command_lock_path", return_value=str(lock_path), ): - self.manager.handle_remote_incomming("command say duplicated") + self.manager.handle_remote_incomming(event_data) mock_environment["runtime"]["OutputManager"].speak_text.assert_not_called() @@ -278,6 +280,24 @@ class TestRemoteDataFormat: assert mock_environment["runtime"]["OutputManager"].speak_text.call_count == 2 + def test_remote_incoming_allows_different_command_after_claim( + self, mock_environment, tmp_path + ): + """Test a fresh different command is not suppressed by the shared lock.""" + self.manager.initialize(mock_environment) + lock_path = tmp_path / "remote-command.lock" + previous_hash = self.manager._get_remote_command_hash("command say first") + lock_path.write_text(f"999999 {previous_hash} {time.time()}\n") + + with patch.object( + self.manager, + "_get_remote_command_lock_path", + return_value=str(lock_path), + ): + self.manager.handle_remote_incomming("command say second") + + mock_environment["runtime"]["OutputManager"].speak_text.assert_called_once() + @pytest.mark.integration @pytest.mark.remote