Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions gui/main_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -1550,8 +1550,8 @@ def _start_calculation(
self._calc_thread.started.connect(self._calc_worker.run)
self._calc_worker.finished.connect(self._on_calculation_finished)
self._calc_worker.failed.connect(self._on_calculation_failed)
self._calc_worker.finished.connect(self._calc_worker.deleteLater)
self._calc_worker.failed.connect(self._calc_worker.deleteLater)
# No deleteLater here: _cleanup_calculation deletes the worker on the
# GUI thread once this thread has stopped.
self._calc_worker.finished.connect(self._calc_thread.quit)
self._calc_worker.failed.connect(self._calc_thread.quit)
self._calc_thread.finished.connect(self._cleanup_calculation)
Expand Down Expand Up @@ -1624,6 +1624,11 @@ def _on_calculation_failed(self, error_message: str) -> None:
def _cleanup_calculation(self) -> None:
if self._calc_thread is not None:
self._calc_thread.deleteLater()
# The worker has no parent, so dropping this last reference deletes it
# here, on the GUI thread, after its thread has stopped. Deleted on its
# own thread instead, its destructor held a Qt signal-slot mutex while
# waiting for the GIL, and the GUI thread could hold the GIL while
# waiting for that same mutex: the app froze.
self._calc_worker = None
self._calc_thread = None
self._calc_active_request = None
Expand Down
33 changes: 33 additions & 0 deletions tests/test_calculated_signal_gui.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from __future__ import annotations

import array
import threading
import time

import pytest
Expand All @@ -24,6 +25,7 @@
from core.signal_store import SignalSeries
from gui.calculated_signal_dialog import (
CalculatedSignalDialog,
CalculationWorker,
_FORMULA_HELP_SIGNAL,
_formula_help_examples,
)
Expand Down Expand Up @@ -215,6 +217,37 @@ def test_large_preflight_can_cancel_before_worker_starts(window, monkeypatch):
assert not window.calculated_signals.contains_key(definition.key)


@pytest.mark.parametrize("outcome", ["finished", "failed"])
def test_calculation_worker_is_deleted_on_the_gui_thread(window, qapp, monkeypatch, outcome):
# Deleted on its own thread, the worker's destructor waits for the GIL while
# holding a Qt signal-slot mutex; the GUI thread, holding the GIL, can be
# waiting for that same mutex. That deadlock froze the app and the suite.
deleted_on = []

class _TrackedWorker(CalculationWorker):
def __init__(self, *args) -> None:
super().__init__(*args)
self.destroyed.connect(
lambda *_: deleted_on.append(threading.get_ident()),
Qt.ConnectionType.DirectConnection,
)

monkeypatch.setattr("gui.main_window.CalculationWorker", _TrackedWorker)
if outcome == "failed":
def _fail(*_args):
raise ValueError("forced failure")

monkeypatch.setattr("gui.calculated_signal_dialog.calculate_series", _fail)
source_key = window.store.all_keys()[0]
definition = CalculatedSignalDefinition("Worker", f"`{source_key}` * 2", "V")

window._queue_calculation(definition, "create", plot_after=False)
_wait_for_calculation(window, qapp)

assert deleted_on == [threading.get_ident()]
assert window.calculated_signals.contains_key(definition.key) is (outcome == "finished")


@pytest.mark.parametrize("mode", ["normal", "multi", "stacked"])
def test_replace_generated_series_preserves_time_and_cursors(qapp, mode):
from gui.plot_widget import PlotPanel
Expand Down
Loading