fix processEvents() reentrancy bug in progress manager window handling (#3030)

* fix progress manager close window race condition

* use monotonic clock for time deltas in progress manager

* restructure progress manager finish logic
This commit is contained in:
Kieran Black 2024-02-26 23:25:53 -06:00 committed by GitHub
parent 60f8399aed
commit 7784321b90
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -3,6 +3,7 @@
from __future__ import annotations from __future__ import annotations
import time import time
from concurrent.futures import Future
from dataclasses import dataclass from dataclasses import dataclass
import aqt.forms import aqt.forms
@ -162,7 +163,7 @@ class ProgressManager:
self._counter = min self._counter = min
self._min = min self._min = min
self._max = max self._max = max
self._firstTime = time.time() self._firstTime = time.monotonic()
self._show_timer = QTimer(self.mw) self._show_timer = QTimer(self.mw)
self._show_timer.setSingleShot(True) self._show_timer.setSingleShot(True)
self._show_timer.start(immediate and 100 or 600) self._show_timer.start(immediate and 100 or 600)
@ -205,7 +206,7 @@ class ProgressManager:
maybeShow: bool = True, maybeShow: bool = True,
max: int | None = None, max: int | None = None,
) -> None: ) -> None:
# print self._min, self._counter, self._max, label, time.time() - self._lastTime # print self._min, self._counter, self._max, label, time.monotonic() - self._lastTime
if not self.mw.inMainThread(): if not self.mw.inMainThread():
print("progress.update() called on wrong thread") print("progress.update() called on wrong thread")
return return
@ -223,8 +224,12 @@ class ProgressManager:
self._win.form.progressBar.setValue(self._counter) self._win.form.progressBar.setValue(self._counter)
def finish(self) -> None: def finish(self) -> None:
# we must latch the levels update and perform it after cleanup has occured or we expose ourselves to a race def do_window_cleanup(future: Future | None = None):
# condition where a second progress could see levels == 0 and wrongly assume everything is in a clean state # this method can be called in an async fashion from taskman where a future
# is passed or in synchronous manner from the main thread
if future is not None:
future.result()
next_levels = self._levels - 1 next_levels = self._levels - 1
next_levels = max(0, next_levels) next_levels = max(0, next_levels)
if next_levels == 0: if next_levels == 0:
@ -243,6 +248,26 @@ class ProgressManager:
self._backend_timer = None self._backend_timer = None
self._levels = next_levels self._levels = next_levels
# if the window is not currently shown, we can do cleanup immediately, if it is
# currently shown then we need to give the window system a half-second to
# present the window before we close it again - fixes progress window getting
# stuck, especially on ubuntu 16.10+
elapsed_time = time.monotonic() - self._shown
time_to_wait = 0.5 - elapsed_time
# NOTE: we must not yield control if the window is not shown since we don't want
# to expose ourselves to the possibility of something showing the window in the
# meantime
if (not self._shown) or (time_to_wait <= 0):
do_window_cleanup()
else:
# NOTE: we can't use self.single_shot here since it won't call the callback
# until _levels = 0, but if we are in this branch then _levels > 0
self.mw.taskman.run_in_background(
lambda time_to_wait=time_to_wait: time.sleep(time_to_wait),
do_window_cleanup,
uses_collection=False,
)
def clear(self) -> None: def clear(self) -> None:
"Restore the interface after an error." "Restore the interface after an error."
if self._levels: if self._levels:
@ -254,25 +279,15 @@ class ProgressManager:
return return
if self._shown: if self._shown:
return return
delta = time.time() - self._firstTime delta = time.monotonic() - self._firstTime
if delta > 0.5: if delta > 0.5:
self._showWin() self._showWin()
def _showWin(self) -> None: def _showWin(self) -> None:
self._shown = time.time() self._shown = time.monotonic()
self._win.show() self._win.show()
def _closeWin(self) -> None: def _closeWin(self) -> None:
if self._shown:
while True:
# give the window system a second to present
# window before we close it again - fixes
# progress window getting stuck, especially
# on ubuntu 16.10+
elap = time.time() - self._shown
if elap >= 0.5:
break
self.app.processEvents(QEventLoop.ProcessEventsFlag.ExcludeUserInputEvents) # type: ignore #possibly related to https://github.com/python/mypy/issues/6910
# if the parent window has been deleted, the progress dialog may have # if the parent window has been deleted, the progress dialog may have
# already been dropped; delete it if it hasn't been # already been dropped; delete it if it hasn't been
if not sip.isdeleted(self._win): if not sip.isdeleted(self._win):