diff --git a/picard/tagger.py b/picard/tagger.py index d71106ea7..058f39f4d 100644 --- a/picard/tagger.py +++ b/picard/tagger.py @@ -318,12 +318,11 @@ class Tagger(QtWidgets.QApplication): self.updatecheckmanager = UpdateCheckManager(parent=self.window) def pipe_server(self): - IGNORED = {pipe.Pipe.MESSAGE_TO_IGNORE, pipe.Pipe.NO_RESPONSE_MESSAGE} + IGNORED = {pipe.Pipe.MESSAGE_TO_IGNORE, pipe.Pipe.NO_RESPONSE_MESSAGE, ""} while self.pipe_running: - messages = self.pipe_handler.read_from_pipe() - for message in messages: - if message not in IGNORED: - self.add_files((message,)) + messages = [x for x in self.pipe_handler.read_from_pipe() if x not in IGNORED] + if messages: + self.add_files(messages) def enable_menu_icons(self, enabled): self.setAttribute(QtCore.Qt.ApplicationAttribute.AA_DontShowIconsInMenus, not enabled) diff --git a/picard/util/pipe.py b/picard/util/pipe.py index 760b34ecf..2248ab434 100644 --- a/picard/util/pipe.py +++ b/picard/util/pipe.py @@ -26,8 +26,8 @@ import concurrent.futures import os from tempfile import NamedTemporaryFile from typing import ( - List, Optional, + Set, ) from picard import ( @@ -166,7 +166,7 @@ class AbstractPipe(metaclass=ABCMeta): def _sender(self, message) -> bool: raise NotImplementedError() - def read_from_pipe(self, timeout_secs: Optional[float] = None) -> List[str]: + def read_from_pipe(self, timeout_secs: Optional[float] = None) -> Set[str]: if timeout_secs is None: timeout_secs = self.TIMEOUT_SECS @@ -175,14 +175,14 @@ class AbstractPipe(metaclass=ABCMeta): try: res = reader.result(timeout=timeout_secs) if res: - out = [r for r in res.split(self.MESSAGE_TO_IGNORE) if r] + out = set([r for r in res.split(self.MESSAGE_TO_IGNORE) if r]) if out: return out except concurrent.futures._base.TimeoutError: # hacky way to kill the file-opening loop self.send_to_pipe(self.MESSAGE_TO_IGNORE) - return [self.NO_RESPONSE_MESSAGE] + return {self.NO_RESPONSE_MESSAGE} def send_to_pipe(self, message: str, timeout_secs: Optional[float] = None) -> bool: if timeout_secs is None: @@ -243,15 +243,23 @@ class UnixPipe(AbstractPipe): return False try: - with open(self.path, 'w') as fifo: + with open(self.path, 'a') as fifo: fifo.write(message) + log.debug("sent successfully: %r", message) + return True except BrokenPipeError: - self.__create_pipe() log.warning("BrokenPipeError happened for %r", message) - log.debug("Re-creating the pipe") - return False + except OSError: + log.warning("append doesn't work, fallback to fifo write mode") + try: + with open(self.path, 'w') as fifo: + fifo.write(message) + log.debug("sent successfully: %r", message) + return True + except BrokenPipeError: + log.warning("BrokenPipeError happened for %r", message) - return True + return False def _reader(self) -> str: response: str = "" @@ -262,12 +270,14 @@ class UnixPipe(AbstractPipe): except FileNotFoundError: raise PipeErrorNotFound from None except BrokenPipeError: - self.__create_pipe() log.warning("BrokenPipeError happened while listening to the pipe") - log.debug("Re-creating the pipe") break - return response or self.NO_RESPONSE_MESSAGE + if response: + log.debug("read value: %r", response) + return response + + return self.NO_RESPONSE_MESSAGE class MacOSPipe(UnixPipe): diff --git a/test/test_util_pipe.py b/test/test_util_pipe.py index 4cf331680..e484b6d41 100644 --- a/test/test_util_pipe.py +++ b/test/test_util_pipe.py @@ -28,25 +28,25 @@ from picard import log from picard.util import pipe -def pipe_listener(pipe_handler, end_of_sequence): - IGNORED_OUTPUT = {pipe.Pipe.MESSAGE_TO_IGNORE, pipe.Pipe.NO_RESPONSE_MESSAGE, "", end_of_sequence} - received = [] - messages = [] +def pipe_listener(pipe_handler): + IGNORED_OUTPUT = {pipe.Pipe.MESSAGE_TO_IGNORE, pipe.Pipe.NO_RESPONSE_MESSAGE, ""} + received = "" - while end_of_sequence not in messages: - messages = pipe_handler.read_from_pipe() - for message in messages: + while not received: + for message in pipe_handler.read_from_pipe(): if message not in IGNORED_OUTPUT: - received.append(message) + received = message + break - return tuple(received) + log.debug("returning: %r", received) + return received -def pipe_writer(pipe_handler, to_send, end_of_sequence): - for message in to_send: - while not pipe_handler.send_to_pipe(message): - pass - while not pipe_handler.send_to_pipe(end_of_sequence): +def pipe_writer(pipe_handler, to_send): + if not to_send: + return False + + while not pipe_handler.send_to_pipe(to_send): pass return True @@ -64,12 +64,12 @@ class TestPipe(PicardTestCase): self.assertRaises(pipe.PipeErrorInvalidAppData, pipe.Pipe, self.NAME, 21, None) def test_pipe_protocol(self): - END_OF_SEQUENCE = "stop" - to_send = ( - ("it", "tests", "picard", "pipe"), - ("test", "number", "two"), - ("my_music_file.mp3",), - ) + to_send = { + "it", "tests", "picard", "pipe", + "test", "number", "two", + "my_music_file.mp3", "last-case", + TestPipe.NAME, TestPipe.VERSION + } pipe_listener_handler = pipe.Pipe(self.NAME, self.VERSION) if pipe_listener_handler.path_was_forced: @@ -78,19 +78,25 @@ class TestPipe(PicardTestCase): pipe_writer_handler = pipe.Pipe(self.NAME, self.VERSION) __pool = concurrent.futures.ThreadPoolExecutor() - - for messages in to_send: - for iteration in range(20): - log.debug("No. %d attempt to send: %r", iteration+1, messages) - plistener = __pool.submit(pipe_listener, pipe_listener_handler, END_OF_SEQUENCE) - pwriter = __pool.submit(pipe_writer, pipe_writer_handler, messages, END_OF_SEQUENCE) - try: - self.assertEqual(plistener.result(timeout=4), messages, - "Data is sent and read correctly") - log.debug("Sent correctly!") - break - except concurrent.futures._base.TimeoutError: + for count in range(100): + for message in to_send: + for iteration in range(20): + log.debug("No. %d attempt to send: %r", iteration+1, message) + plistener = __pool.submit(pipe_listener, pipe_listener_handler) + pwriter = __pool.submit(pipe_writer, pipe_writer_handler, message) + to_break = False try: - pwriter.result(timeout=4) + self.assertEqual(plistener.result(timeout=6.5), message, + "Data is sent and read correctly") + log.debug("Sent correctly!") + to_break = True except concurrent.futures._base.TimeoutError: - pass + pipe_writer_handler.send_to_pipe(pipe_writer_handler.MESSAGE_TO_IGNORE) + + try: + pwriter.result(timeout=0.01) + except concurrent.futures._base.TimeoutError: + pipe_listener_handler.read_from_pipe() + + if to_break: + break