From af2c748127be6b3ed45d3c1c7c8f32008e98507c Mon Sep 17 00:00:00 2001 From: wyzula-jan Date: Fri, 16 Oct 2026 16:38:34 +0200 Subject: [PATCH] fix(pdf_viewer): connect document status once instead of disconnecting unconnected slots --- .../widgets/utility/pdf_viewer/pdf_viewer.py | 16 +++++--------- tests/unit_tests/test_pdf_viewer.py | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/bec_widgets/widgets/utility/pdf_viewer/pdf_viewer.py b/bec_widgets/widgets/utility/pdf_viewer/pdf_viewer.py index 8e88cbc2..ab1da6f7 100644 --- a/bec_widgets/widgets/utility/pdf_viewer/pdf_viewer.py +++ b/bec_widgets/widgets/utility/pdf_viewer/pdf_viewer.py @@ -56,6 +56,7 @@ class PdfViewerWidget(BECWidget, QWidget): # Create the PDF document and view first self._pdf_document = QPdfDocument(self) + self._pdf_document.statusChanged.connect(self._on_document_status_changed) self.pdf_view = QPdfView() self.pdf_view.setZoomMode(QPdfView.ZoomMode.FitToWidth) @@ -328,15 +329,6 @@ class PdfViewerWidget(BECWidget, QWidget): self._current_file_path = file_path - # Disconnect any existing signal connections - try: - self._pdf_document.statusChanged.disconnect(self._on_document_status_changed) - except (TypeError, RuntimeError): - pass - - # Connect to statusChanged signal to handle when document is ready - self._pdf_document.statusChanged.connect(self._on_document_status_changed) - # Load the document self._pdf_document.load(file_path) @@ -544,10 +536,12 @@ class PdfViewerWidget(BECWidget, QWidget): def cleanup(self): """Handle widget close event to prevent segfaults.""" - if hasattr(self, "_pdf_document") and self._pdf_document: - self._pdf_document.statusChanged.disconnect() + # Cleanup can run twice (explicit call, then close); disconnect only once. + if getattr(self, "_pdf_document", None) is not None: + self._pdf_document.statusChanged.disconnect(self._on_document_status_changed) empty_doc = QPdfDocument(self) self.pdf_view.setDocument(empty_doc) + self._pdf_document = None if hasattr(self, "toolbar"): self.toolbar.cleanup() diff --git a/tests/unit_tests/test_pdf_viewer.py b/tests/unit_tests/test_pdf_viewer.py index 61cf9186..bcda73f8 100644 --- a/tests/unit_tests/test_pdf_viewer.py +++ b/tests/unit_tests/test_pdf_viewer.py @@ -1,3 +1,5 @@ +import warnings + import pytest from qtpy.QtPdf import QPdfDocument from qtpy.QtPdfWidgets import QPdfView @@ -227,6 +229,26 @@ def test_load_pdf_file(qtbot, pdf_viewer_widget: PdfViewerWidget, temp_pdf_file, assert widget.current_file_path == temp_pdf_file +def test_load_and_cleanup_do_not_warn_about_disconnect( + qtbot, mocked_client, temp_pdf_file, temp_pdf_file_2 +): + """Loading, reloading and (repeated) cleanup never disconnect a slot that is not connected.""" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + widget = PdfViewerWidget(client=mocked_client) + qtbot.addWidget(widget) + widget.load_pdf(temp_pdf_file) + widget.load_pdf(temp_pdf_file_2) + qtbot.wait(100) + widget.cleanup() + widget.cleanup() # a repeated cleanup must not disconnect again + unloaded = PdfViewerWidget(client=mocked_client) + qtbot.addWidget(unloaded) + unloaded.cleanup() + + assert [str(w.message) for w in caught if "Failed to disconnect" in str(w.message)] == [] + + def test_load_invalid_pdf_file(qtbot, pdf_viewer_widget: PdfViewerWidget, tmpdir): """Test loading an invalid PDF file into the viewer.""" widget = pdf_viewer_widget