Use single shared remote command lock
This commit is contained in:
@@ -39,7 +39,7 @@ from fenrirscreenreader.core.i18n import _
|
|||||||
|
|
||||||
|
|
||||||
REMOTE_COMMAND_LOCK_TIMEOUT_SEC = 2.0
|
REMOTE_COMMAND_LOCK_TIMEOUT_SEC = 2.0
|
||||||
REMOTE_COMMAND_LOCK_PREFIX = "fenrirscreenreader-remote-"
|
REMOTE_COMMAND_LOCK_FILE = "fenrirscreenreader-remote-command.lock"
|
||||||
|
|
||||||
|
|
||||||
class RemoteManager:
|
class RemoteManager:
|
||||||
@@ -282,28 +282,10 @@ class RemoteManager:
|
|||||||
return
|
return
|
||||||
|
|
||||||
def _get_remote_command_lock_path(self, event_data):
|
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(), REMOTE_COMMAND_LOCK_FILE)
|
||||||
return os.path.join(
|
|
||||||
tempfile.gettempdir(),
|
|
||||||
f"{REMOTE_COMMAND_LOCK_PREFIX}{event_hash}.lock",
|
|
||||||
)
|
|
||||||
|
|
||||||
def _cleanup_stale_remote_command_locks(self, now):
|
def _get_remote_command_hash(self, event_data):
|
||||||
try:
|
return hashlib.sha256(event_data.encode("utf-8")).hexdigest()
|
||||||
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 _read_remote_command_lock(self, lock_file):
|
def _read_remote_command_lock(self, lock_file):
|
||||||
lock_file.seek(0)
|
lock_file.seek(0)
|
||||||
@@ -312,12 +294,13 @@ class RemoteManager:
|
|||||||
lock_pid = int(lock_parts[0]) if lock_parts else 0
|
lock_pid = int(lock_parts[0]) if lock_parts else 0
|
||||||
except ValueError:
|
except ValueError:
|
||||||
lock_pid = 0
|
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.seek(0)
|
||||||
lock_file.truncate()
|
lock_file.truncate()
|
||||||
lock_file.write(f"{os.getpid()} {now}\n")
|
lock_file.write(f"{os.getpid()} {event_hash} {now}\n")
|
||||||
lock_file.flush()
|
lock_file.flush()
|
||||||
os.fsync(lock_file.fileno())
|
os.fsync(lock_file.fileno())
|
||||||
|
|
||||||
@@ -335,8 +318,8 @@ class RemoteManager:
|
|||||||
|
|
||||||
def _claim_remote_command(self, event_data):
|
def _claim_remote_command(self, event_data):
|
||||||
lock_path = self._get_remote_command_lock_path(event_data)
|
lock_path = self._get_remote_command_lock_path(event_data)
|
||||||
|
event_hash = self._get_remote_command_hash(event_data)
|
||||||
now = time.time()
|
now = time.time()
|
||||||
self._cleanup_stale_remote_command_locks(now)
|
|
||||||
lock_file = self._open_remote_command_lock(lock_path)
|
lock_file = self._open_remote_command_lock(lock_path)
|
||||||
if lock_file is None:
|
if lock_file is None:
|
||||||
return False
|
return False
|
||||||
@@ -344,20 +327,21 @@ class RemoteManager:
|
|||||||
with lock_file:
|
with lock_file:
|
||||||
try:
|
try:
|
||||||
fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX)
|
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())
|
lock_stat = os.fstat(lock_file.fileno())
|
||||||
|
|
||||||
if lock_pid == os.getpid():
|
if lock_pid == os.getpid():
|
||||||
|
self._write_remote_command_lock(lock_file, event_hash, now)
|
||||||
return True
|
return True
|
||||||
|
|
||||||
if not lock_pid:
|
if not lock_pid or lock_hash != event_hash:
|
||||||
self._write_remote_command_lock(lock_file, now)
|
self._write_remote_command_lock(lock_file, event_hash, now)
|
||||||
return True
|
return True
|
||||||
|
|
||||||
if now - lock_stat.st_mtime < REMOTE_COMMAND_LOCK_TIMEOUT_SEC:
|
if now - lock_stat.st_mtime < REMOTE_COMMAND_LOCK_TIMEOUT_SEC:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
self._write_remote_command_lock(lock_file, now)
|
self._write_remote_command_lock(lock_file, event_hash, now)
|
||||||
return True
|
return True
|
||||||
finally:
|
finally:
|
||||||
fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)
|
fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)
|
||||||
|
|||||||
@@ -250,14 +250,16 @@ class TestRemoteDataFormat:
|
|||||||
"""Test untargeted duplicate remote commands only run in one process."""
|
"""Test untargeted duplicate remote commands only run in one process."""
|
||||||
self.manager.initialize(mock_environment)
|
self.manager.initialize(mock_environment)
|
||||||
lock_path = tmp_path / "remote-command.lock"
|
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(
|
with patch.object(
|
||||||
self.manager,
|
self.manager,
|
||||||
"_get_remote_command_lock_path",
|
"_get_remote_command_lock_path",
|
||||||
return_value=str(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()
|
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
|
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.integration
|
||||||
@pytest.mark.remote
|
@pytest.mark.remote
|
||||||
|
|||||||
Reference in New Issue
Block a user