[FIX] Fix OWWidget destruction

Orange/canvas/scheme/widgetsscheme.py

@@ -511,6 +511,13 @@ def _delete_widget(self, widget):
             self.__delay_delete.add(widget)
         else:
             widget.deleteLater()
+            name = "{} '{}'".format(type(widget).__name__, widget.captionTitle)
+            if log.isEnabledFor(logging.DEBUG):
+                widget.destroyed.connect(
+                    lambda: log.debug("Destroyed: %s", name))
+                widget.__marker = QObject()
+                widget.__marker.destroyed.connect(
+                    lambda: log.debug("Destroyed namespace: %s", name))
             del self.__widget_processing_state[widget]
 
     def create_widget_instance(self, node):
@@ -953,7 +960,7 @@ def __set_float_on_top_flag(self, widget):
 def user_message_from_state(message_group):
     return UserMessage(
         severity=message_group.severity,
-        message_id=message_group,
+        message_id="{0.__name__}.{0.__qualname__}".format(type(message_group)),
         contents="<br/>".join(msg.formatted
                               for msg in message_group.active) or None,
         data={"content-type": "text/html"})

Orange/widgets/tests/test_widget.py

@@ -1,10 +1,15 @@
 # Test methods with long descriptive names can omit docstrings
-# pylint: disable=missing-docstring
+# pylint: disable=all
+
+import gc
+import weakref
 
 from unittest.mock import patch, MagicMock
-from AnyQt.QtCore import QRect, QByteArray
+
+from AnyQt.QtCore import QRect, QByteArray, QObject, pyqtSignal
 from AnyQt.QtGui import QShowEvent
 from AnyQt.QtWidgets import QAction
+from AnyQt.QtTest import QSignalSpy
 
 from Orange.widgets.gui import OWComponent
 from Orange.widgets.settings import Setting
@@ -87,6 +92,64 @@ class TestWidget2(OWWidget):
         w = TestWidget2()
         w.showEvent(QShowEvent())
 
+    def test_store_restore_layout_geom(self):
+        class Widget(OWWidget):
+            name = "Who"
+            want_control_area = True
+
+        w = Widget()
+        w._OWWidget__setControlAreaVisible(False)
+        w.setGeometry(QRect(51, 52, 53, 54))
+        state = w.saveGeometryAndLayoutState()
+        w1 = Widget()
+        self.assertTrue(w1.restoreGeometryAndLayoutState(state))
+        self.assertEqual(w1.geometry(), QRect(51, 52, 53, 54))
+        self.assertFalse(w1.controlAreaVisible)
+
+        Widget.want_control_area = False
+        w2 = Widget()
+        self.assertTrue(w2.restoreGeometryAndLayoutState(state))
+        self.assertEqual(w1.geometry(), QRect(51, 52, 53, 54))
+
+        self.assertFalse((w2.restoreGeometryAndLayoutState(QByteArray())))
+        self.assertFalse(w2.restoreGeometryAndLayoutState(QByteArray(b'ab')))
+
+    def test_garbage_collect(self):
+        widget = MyWidget()
+        ref = weakref.ref(widget)
+        # insert an object in widget's __dict__ that will be deleted when its
+        # __dict__ is cleared.
+        widget._finalizer = QObject()
+        spyw = DestroyedSignalSpy(widget)
+        spyf = DestroyedSignalSpy(widget._finalizer)
+        widget.deleteLater()
+        del widget
+        gc.collect()
+        self.assertTrue(len(spyw) == 1 or spyw.wait(1000))
+        gc.collect()
+        self.assertTrue(len(spyf) == 1 or spyf.wait(1000))
+        gc.collect()
+        self.assertIsNone(ref())
+
+    def test_garbage_collect_from_scheme(self):
+        from Orange.canvas.scheme.widgetsscheme import WidgetsScheme
+        from Orange.canvas.registry.description import WidgetDescription
+        new_scheme = WidgetsScheme()
+        w_desc = WidgetDescription.from_module("Orange.widgets.tests.test_widget")
+        node = new_scheme.new_node(w_desc)
+        widget = new_scheme.widget_for_node(node)
+        widget._finalizer = QObject()
+        spyw = DestroyedSignalSpy(widget)
+        spyf = DestroyedSignalSpy(widget._finalizer)
+        ref = weakref.ref(widget)
+        del widget
+        new_scheme.remove_node(node)
+        gc.collect()
+        self.assertTrue(len(spyw) == 1 or spyw.wait(1000))
+        gc.collect()
+        self.assertTrue(len(spyf) == 1 or spyf.wait(1000))
+        self.assertIsNone(ref())
+
 
 class WidgetMsgTestCase(WidgetTest):
 
@@ -188,24 +251,21 @@ def test_old_style_messages(self):
         w.Error.clear()
         self.assertEqual(len(messages), 0)
 
-    def test_store_restore_layout_geom(self):
-        class Widget(OWWidget):
-            name = "Who"
-            want_control_area = True
 
-        w = Widget()
-        w._OWWidget__setControlAreaVisible(False)
-        w.setGeometry(QRect(51, 52, 53, 54))
-        state = w.saveGeometryAndLayoutState()
-        w1 = Widget()
-        self.assertTrue(w1.restoreGeometryAndLayoutState(state))
-        self.assertEqual(w1.geometry(), QRect(51, 52, 53, 54))
-        self.assertFalse(w1.controlAreaVisible)
+class DestroyedSignalSpy(QSignalSpy):
+    """
+    A signal spy for watching QObject.destroyed signal
 
-        Widget.want_control_area = False
-        w2 = Widget()
-        self.assertTrue(w2.restoreGeometryAndLayoutState(state))
-        self.assertEqual(w1.geometry(), QRect(51, 52, 53, 54))
+    NOTE: This class specifically does not capture the QObject pointer emitted
+    from the destroyed signal (i.e. it connects to the no arg overload).
+    """
+    class Mapper(QObject):
+        destroyed_ = pyqtSignal()
 
-        self.assertFalse((w2.restoreGeometryAndLayoutState(QByteArray())))
-        self.assertFalse(w2.restoreGeometryAndLayoutState(QByteArray(b'ab')))
+    def __init__(self, obj):
+        # type: (QObject) -> None
+        # Route the signal via a no argument signal to drop the obj pointer.
+        # After the destroyed signal is emitted the pointer is invalid
+        self.__mapper = DestroyedSignalSpy.Mapper()
+        obj.destroyed.connect(self.__mapper.destroyed_)
+        super().__init__(self.__mapper.destroyed_)

Orange/widgets/widget.py

@@ -221,6 +221,7 @@ def __new__(cls, *args, captionTitle=None, **kwargs):
 
         self.left_side = None
         self.controlArea = self.mainArea = self.buttonsArea = None
+        self.__progressBar = None
         self.__splitter = None
         if self.want_basic_layout:
             self.set_basic_layout()
@@ -426,33 +427,36 @@ def _():
             self.message_bar = MessagesWidget(self)
             self.message_bar.setSizePolicy(QSizePolicy.Preferred,
                                            QSizePolicy.Preferred)
-            pb = QProgressBar(maximumWidth=120, minimum=0, maximum=100)
+            self.__progressBar = pb = QProgressBar(
+                maximumWidth=120, minimum=0, maximum=100
+            )
             pb.setSizePolicy(QSizePolicy.Maximum, QSizePolicy.Ignored)
             pb.setAttribute(Qt.WA_LayoutUsesWidgetRect)
             pb.setAttribute(Qt.WA_MacMiniSize)
             pb.hide()
             sb.addPermanentWidget(pb)
             sb.addPermanentWidget(self.message_bar)
 
-            def statechanged():
-                pb.setVisible(bool(self.processingState) or self.isBlocking())
-                if self.isBlocking() and not self.processingState:
-                    pb.setRange(0, 0)  # indeterminate pb
-                elif self.processingState:
-                    pb.setRange(0, 100)  # determinate pb
-
-            self.processingStateChanged.connect(statechanged)
-            self.blockingStateChanged.connect(statechanged)
-
-            @self.progressBarValueChanged.connect
-            def _(val):
-                pb.setValue(int(val))
+            self.processingStateChanged.connect(self.__processingStateChanged)
+            self.blockingStateChanged.connect(self.__processingStateChanged)
+            self.progressBarValueChanged.connect(lambda v: pb.setValue(int(v)))
 
             # Reserve the bottom margins for the status bar
             margins = self.layout().contentsMargins()
             margins.setBottom(sb.sizeHint().height())
             self.setContentsMargins(margins)
 
+    def __processingStateChanged(self):
+        # Update the progress bar in the widget's status bar
+        pb = self.__progressBar
+        if pb is None:
+            return
+        pb.setVisible(bool(self.processingState) or self.isBlocking())
+        if self.isBlocking() and not self.processingState:
+            pb.setRange(0, 0)  # indeterminate pb
+        elif self.processingState:
+            pb.setRange(0, 100)  # determinate pb
+
     def __toggleControlArea(self):
         if self.__splitter is None or self.__splitter.count() < 2:
             return