From 598226a5c0c1f857280047e5d77b846c2afddf5d Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Fri, 21 Feb 2020 15:14:09 +1000 Subject: [PATCH] possible fix for race conditions in the sound code https://anki.tenderapp.com/discussions/ankidesktop/39030-erro-ao-adicionar-arquivo-de-udio the lock should at least ensure _process doesn't disappear in the middle of our logic, and the longer wait should reduce the chances of .stop() timing out and allowing multiple audio files to play Not very happy with the current approach, as in the timeout case you have multiple threads competing to access the same data --- qt/aqt/sound.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/qt/aqt/sound.py b/qt/aqt/sound.py index 3bbcfaba6..0b0c5be8c 100644 --- a/qt/aqt/sound.py +++ b/qt/aqt/sound.py @@ -229,6 +229,7 @@ class SimpleProcessPlayer(Player): # pylint: disable=abstract-method self._taskman = taskman self._terminate_flag = False self._process: Optional[subprocess.Popen] = None + self._lock = threading.Lock() def play(self, tag: AVTag, on_done: OnDoneCallback) -> None: self._taskman.run_in_background( @@ -239,7 +240,7 @@ class SimpleProcessPlayer(Player): # pylint: disable=abstract-method self._terminate_flag = True # block until stopped t = time.time() - while self._terminate_flag and time.time() - t < 1: + while self._terminate_flag and time.time() - t < 3: time.sleep(0.1) def _play(self, tag: AVTag) -> None: @@ -254,21 +255,32 @@ class SimpleProcessPlayer(Player): # pylint: disable=abstract-method lambda: gui_hooks.av_player_did_begin_playing(self, tag) ) - try: - while True: + while True: + with self._lock: + # if .stop() timed out, another thread may run when + # there is no process + if not self._process: + self._process = None + self._terminate_flag = False + return + + # should we abort playing? + if self._terminate_flag: + self._process.terminate() + self._process = None + self._terminate_flag = False + raise PlayerInterrupted() + + # wait for completion try: self._process.wait(0.1) if self._process.returncode != 0: print(f"player got return code: {self._process.returncode}") + self._process = None + self._terminate_flag = False return except subprocess.TimeoutExpired: pass - if self._terminate_flag: - self._process.terminate() - raise PlayerInterrupted() - finally: - self._process = None - self._terminate_flag = False def _on_done(self, ret: Future, cb: OnDoneCallback) -> None: try: