fix/fallback_timeout_handling (#210)

* fix/fallback_timeout_handling

by using a killable_event we can abort fallbacks that timedout avoiding multiple answers

closes OpenVoiceOS/ovos-core#389

also avoids sessions being able to kill each other's events, could be problematic in get_response and company

* fix tests

* fix tests

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:

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}.killed",
+            data={"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

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"],

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())