Fix #1323 for the Projector Manager

openlp/core/projectors/manager.py

@@ -23,8 +23,8 @@
 
 Provides the functions for the display/control of Projectors.
 """
-
 import logging
+from typing import Optional
 
 from PyQt5 import QtCore, QtGui, QtWidgets
 
@@ -305,6 +305,12 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
             E_UNKNOWN_SOCKET_ERROR: UiIcons().error,
             E_NOT_CONNECTED: UiIcons().projector_disconnect
         }
+        # update_status debouncer
+        self.update_status_timer = QtCore.QTimer(self)
+        self.update_status_timer.setInterval(100)
+        self.update_status_timer.timeout.connect(self._try_update_status)
+        self.is_updating_status = False
+        self.ip_status_to_update = (None, None, None)
 
     def bootstrap_initialise(self):
         """
@@ -810,7 +816,7 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
         return self.projector_list
 
     @QtCore.pyqtSlot(str, int, str)
-    def update_status(self, ip, status=None, msg=None):
+    def update_status(self, ip: str, status: Optional[int] = None, msg: Optional[str] = None):
         """
         Update the status information/icon for selected list item
 
@@ -820,23 +826,51 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
         """
         if status is None:
             return
-        item = None
-        for list_item in self.projector_list:
-            if ip == list_item.link.ip:
-                item = list_item
-                break
-        if item is None:
-            log.error(f'ProjectorManager: Unknown item "{ip}" - not updating status')
-            return
-        elif item.status == status:
-            log.debug(f'ProjectorManager: No status change for "{ip}" - not updating status')
-            return
+        self._try_update_status(ip, status, msg)
 
-        item.status = status
-        item.icon = self.status_icons[status]
-        log.debug(f'({item.link.name}) Updating icon with {STATUS_CODE[status]}')
-        item.widget.setIcon(item.icon)
-        return self.update_icons()
+    def _try_update_status(self, ip: str, status: int, msg: Optional[str] = None):
+        """
+        Try to update the status of a projector
+        """
+        if not self.is_updating_status:
+            self.update_status_timer.stop()
+            self._update_status(ip, status, msg)
+        else:
+            self.update_status_timer.stop()
+            self.update_status_timer.start()
+
+    def _update_status(self, ip: str, status: int, msg: Optional[str] = None):
+        """
+        Actually update the status of the projector
+        """
+        self.is_updating_status = True
+        try:
+            item = None
+            for list_item in self.projector_list:
+                if ip == list_item.link.ip:
+                    item = list_item
+                    break
+            if item is None:
+                log.error(f'ProjectorManager: Unknown item "{ip}" - not updating status')
+                self.is_updating_status = False
+                return
+            elif item.status == status:
+                log.debug(f'ProjectorManager: No status change for "{ip}" - not updating status')
+                self.is_updating_status = False
+                return
+
+            item.status = status
+            item.icon = self.status_icons[status]
+            log.debug(f'({item.link.name}) Updating icon with {STATUS_CODE[status]}')
+            item.widget.setIcon(item.icon)
+            self.is_updating_status = False
+            return self.update_icons()
+        except RuntimeError:
+            # it's probably a "wrapped C/C++ object of type QTreeWidgetItem has been deleted" due to
+            # consecutive/parallel repaint_service_list execution. We've added some mitigation to avoid this
+            # to happen, but it for any reason it happens again, we'll silent it and try to repaint the list
+            # again (to avoid a broken list presented to the user).
+            self.is_updating_status = False
 
     def get_toolbar_item(self, name, enabled=False, hidden=False):
         item = self.one_toolbar.findChild(QtWidgets.QAction, name)

tests/openlp_core/projectors/manager/test_misc_manager.py

@@ -26,9 +26,14 @@ add_projector_from_wizard()
 get_projector_list()
 
 """
-from unittest.mock import DEFAULT, patch
+from threading import Thread
+from time import sleep
+from typing import Optional
+from unittest.mock import DEFAULT, MagicMock, patch
 
+from openlp.core.common.registry import Registry
 from openlp.core.projectors.db import Projector
+from openlp.core.projectors.manager import ProjectorManager
 
 from tests.resources.projector.data import TEST1_DATA, TEST2_DATA, TEST3_DATA
 
@@ -118,3 +123,27 @@ def test_get_projector_list(projector_manager_mtdb):
     for dbitem in t_chk:
         t_chk_list.append(dbitem.db_item)
     assert t_list == t_chk_list, 'projector_list DB items do not match test items'
+
+
+def test_update_status_call_not_parallel(registry: Registry, projector_manager_nodb: ProjectorManager):
+    """
+    Tests if _replace_service_list calls are not done in parallel
+    """
+    # GIVEN a service manager and a mocked repaint
+    def mock_update_status(ip: str, status: int, msg: Optional[str] = None):
+        projector_manager_nodb.is_updating_status = True
+        sleep(0.25)
+        projector_manager_nodb.is_updating_status = False
+
+    projector_manager_nodb._update_status = MagicMock(side_effect=mock_update_status)
+
+    # WHEN repaint_service_list is called from different threads
+    thread_1 = Thread(target=lambda: projector_manager_nodb.update_status('192.168.88.238', 1))
+    thread_2 = Thread(target=lambda: projector_manager_nodb.update_status('192.168.88.238', 2))
+    thread_1.start()
+    thread_2.start()
+
+    # THEN the _repaint_service_list call must be called only once
+    thread_1.join()
+    thread_2.join()
+    projector_manager_nodb._update_status.assert_called_once()