Skip to content

Commit

Permalink
PICARD-2668: Ensure application quit() runs all exit() cleanup tasks
Browse files Browse the repository at this point in the history
This avoids issues like pipe handler hanging with waiting threads after
main application crashed.
  • Loading branch information
phw committed Jul 2, 2023
1 parent ea9e613 commit 9928c09
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 11 deletions.
37 changes: 27 additions & 10 deletions picard/tagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,17 @@ class Tagger(QtWidgets.QApplication):
_no_restore = False

def __init__(self, picard_args, localedir, autoupdate, pipe_handler=None):
# Initialize these variables early as they are needed for a clean
# shutdown.
self._acoustid = None
self.browser_integration = None
self.exit_cleanup = []
self.pipe_handler = None
self.priority_thread_pool = None
self.save_thread_pool = None
self.stopping = False
self.thread_pool = None
self.webservice = None

super().__init__(sys.argv)
self.__class__.__instance = self
Expand Down Expand Up @@ -359,8 +370,6 @@ def __init__(self, picard_args, localedir, autoupdate, pipe_handler=None):
self.unclustered_files = UnclusteredFiles()
self.nats = None
self.window = MainWindow(disable_player=picard_args.no_player)
self.exit_cleanup = []
self.stopping = False

# On macOS temporary files get deleted after 3 days not being accessed.
# Touch these files regularly to keep them alive if Picard
Expand Down Expand Up @@ -549,7 +558,6 @@ def handle_command_pause(self, argstring):

def handle_command_quit(self, argstring):
if argstring.upper() == 'FORCE' or self.window.show_quit_confirmation():
self.exit()
self.quit()
else:
log.info("QUIT command cancelled by the user.")
Expand Down Expand Up @@ -699,19 +707,29 @@ def move_file_to_nat(self, file, recordingid, node=None):
if nat.loaded:
self.nats.update()

def quit(self):
self.exit()
super().quit()

def exit(self):
if self.stopping:
return
self.stopping = True
log.debug("Picard stopping")
self._acoustid.done()
if self._acoustid:
self._acoustid.done()
if self.pipe_handler:
self.pipe_handler.stop()
self.webservice.stop()
self.thread_pool.waitForDone()
self.save_thread_pool.waitForDone()
self.priority_thread_pool.waitForDone()
self.browser_integration.stop()
if self.webservice:
self.webservice.stop()
if self.thread_pool:
self.thread_pool.waitForDone()
if self.save_thread_pool:
self.save_thread_pool.waitForDone()
if self.priority_thread_pool:
self.priority_thread_pool.waitForDone()
if self.browser_integration:
self.browser_integration.stop()
self.run_cleanup()
QtCore.QCoreApplication.processEvents()

Expand Down Expand Up @@ -1307,7 +1325,6 @@ def signal(self, signum, frame):

def sighandler(self):
self.signalnotifier.setEnabled(False)
self.exit()
self.quit()
self.signalnotifier.setEnabled(True)

Expand Down
2 changes: 1 addition & 1 deletion picard/util/pipe.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ def send_to_pipe(self, message: str, timeout_secs: Optional[float] = None) -> bo
if timeout_secs is None:
timeout_secs = self.TIMEOUT_SECS

with concurrent.futures.ThreadPoolExecutor() as executor:
with concurrent.futures.ThreadPoolExecutor(max_workers=1) as executor:
# we're sending only filepaths, so we have to create some kind of separator
# to avoid any potential conflicts and mixing the data
sender = executor.submit(self._sender, message + self.MESSAGE_TO_IGNORE)
Expand Down

0 comments on commit 9928c09

Please sign in to comment.