From cd5d610e892006b318461b30823d28bb13a36a23 Mon Sep 17 00:00:00 2001 From: miro Date: Tue, 18 Jun 2024 03:50:09 +0100 Subject: [PATCH 1/3] fix/fallback_timeout_handling by using a killable_event we can abort fallbacks that timedout avoiding multiple answers closes https://github.com/OpenVoiceOS/ovos-core/issues/389 also avoids sessions being able to kill each other's events, could be problematic in get_response and company --- ovos_workshop/decorators/killable.py | 16 ++++++++++++---- ovos_workshop/skills/fallback.py | 10 +++++++++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/ovos_workshop/decorators/killable.py b/ovos_workshop/decorators/killable.py index 20df6fc7..871aaf1f 100644 --- a/ovos_workshop/decorators/killable.py +++ b/ovos_workshop/decorators/killable.py @@ -1,10 +1,12 @@ +import threading +from functools import wraps +from inspect import signature from typing import Optional, Type +from ovos_bus_client.session import SessionManager from ovos_utils import create_killable_daemon from ovos_utils.fakebus import Message -import threading -from inspect import signature -from functools import wraps +from ovos_utils.log import LOG class AbortEvent(StopIteration): @@ -56,10 +58,16 @@ def create_killable(func): def call_function(*args, **kwargs): skill = args[0] t = create_killable_daemon(func, args, kwargs, autostart=False) + sess = SessionManager.get() - def abort(_): + def abort(m: Message): if not t.is_alive(): return + # check if session matches (dont kill events from other sessions) + sess2 = SessionManager.get(m) + if sess.session_id != sess2.session_id: + LOG.debug(f"ignoring '{msg}' kill event, event listener not created by this session") + return if stop_tts: skill.bus.emit(Message("mycroft.audio.speech.stop")) if call_stop: diff --git a/ovos_workshop/skills/fallback.py b/ovos_workshop/skills/fallback.py index 3cdd1aff..3297dca7 100644 --- a/ovos_workshop/skills/fallback.py +++ b/ovos_workshop/skills/fallback.py @@ -16,13 +16,14 @@ from typing import Optional, List, Callable, Tuple from ovos_bus_client import MessageBusClient -from ovos_bus_client.message import Message +from ovos_bus_client.message import Message, dig_for_message from ovos_config import Configuration from ovos_utils.events import get_handler_name from ovos_utils.log import LOG from ovos_utils.metrics import Stopwatch from ovos_utils.skills import get_non_properties +from ovos_workshop.decorators.killable import killable_event from ovos_workshop.decorators.compat import backwards_compat from ovos_workshop.permissions import FallbackMode from ovos_workshop.skills.ovos import OVOSSkill @@ -356,6 +357,13 @@ def _handle_fallback_ack(self, message: Message): "can_handle": self.can_answer(utts, lang)}, context={"skill_id": self.skill_id})) + def _on_timeout(self): + message = dig_for_message() + self.bus.emit(message.forward( + f"ovos.skills.fallback.{self.skill_id}.response", + data={"result": False, "error": "timed out"})) + + @killable_event("ovos.skills.fallback.force_timeout", callback=_on_timeout) def _handle_fallback_request(self, message: Message): """ Handle a fallback request, calling any registered handlers in priority From 21e91318de3dcbc6957d8e5a79256256159c4062 Mon Sep 17 00:00:00 2001 From: miro Date: Tue, 18 Jun 2024 04:00:56 +0100 Subject: [PATCH 2/3] fix tests --- test/unittests/skills/test_base.py | 2 +- test/unittests/skills/test_fallback_skill.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/test/unittests/skills/test_base.py b/test/unittests/skills/test_base.py index d3f2944f..c81d3ef3 100644 --- a/test/unittests/skills/test_base.py +++ b/test/unittests/skills/test_base.py @@ -145,7 +145,7 @@ def _update_skill_settings(): stop_time = time() stop_event.set() thread.join() - self.assertAlmostEquals(self.skill.settings["test_val"], stop_time, + self.assertAlmostEqual(self.skill.settings["test_val"], stop_time, 0, f"run {i}") self.assertNotEqual(self.skill.settings["test_val"], self.skill._initial_settings["test_val"], diff --git a/test/unittests/skills/test_fallback_skill.py b/test/unittests/skills/test_fallback_skill.py index 7320d791..656ab4da 100644 --- a/test/unittests/skills/test_fallback_skill.py +++ b/test/unittests/skills/test_fallback_skill.py @@ -1,3 +1,4 @@ +import time from unittest import TestCase from unittest.mock import patch, Mock @@ -262,6 +263,8 @@ def mock_resonse(message: Message): self.fallback_skill._fallback_handlers = [(100, mock_handler)] self.fallback_skill._handle_fallback_request(Message("test")) + time.sleep(0.2) # above runs in a killable thread + self.assertTrue(start_event.is_set()) self.assertTrue(handler_event.is_set()) From b824b90436a78686a25587c4fc6a7390835db68c Mon Sep 17 00:00:00 2001 From: miro Date: Tue, 18 Jun 2024 04:27:04 +0100 Subject: [PATCH 3/3] fix tests --- ovos_workshop/skills/fallback.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ovos_workshop/skills/fallback.py b/ovos_workshop/skills/fallback.py index 3297dca7..68f6de43 100644 --- a/ovos_workshop/skills/fallback.py +++ b/ovos_workshop/skills/fallback.py @@ -360,8 +360,8 @@ def _handle_fallback_ack(self, message: Message): def _on_timeout(self): message = dig_for_message() self.bus.emit(message.forward( - f"ovos.skills.fallback.{self.skill_id}.response", - data={"result": False, "error": "timed out"})) + f"ovos.skills.fallback.{self.skill_id}.killed", + data={"error": "timed out"})) @killable_event("ovos.skills.fallback.force_timeout", callback=_on_timeout) def _handle_fallback_request(self, message: Message):