fix(remote): stale results and deadlock when closing during a call (#398)

- A remote command returning None (e.g. get_current_object_uuid with no
  current object) returned the result of the previous command instead,
  such as the panel name returned by call_method: the result is now
  reset at the start of each request, like the exception.
- Closing DataLab while a remote call was waiting for it to be ready
  (e.g. call_method("close")) hung DataLab: the main window, blocked
  until the server stopped, could never signal that the request was
  done. Shutting down the server now releases the waiting call first.

Both are covered by new tests, the closing one failing instead of
hanging should the deadlock come back.

Assisted-by: Claude Opus 5.5

Author: PierreRaybaut

datalab/control/remote.py

@@ -51,6 +51,7 @@ def method_wrapper(*args, **kwargs):
         self: RemoteServer = args[0]  # extracting 'self' from method arguments
         self.is_ready = False
         self.exception = None
+        self.result = None
         output = func(*args, **kwargs)
         while not self.is_ready:
             QC.QCoreApplication.processEvents()
@@ -179,6 +180,10 @@ def shutdown_server(self) -> None:
         """Shutdown server"""
         # A server still waiting for the main window must be able to stop
         self.__window_ready.set()
+        # Release the remote call waiting for DataLab to be ready, if any (e.g. the
+        # one closing DataLab): the main window, blocked below until the request is
+        # done, could not signal it, hence a deadlock
+        self.is_ready = True
         if self.server is not None:
             self.server.shutdown()
             self.server = None

datalab/tests/features/control/remote_requests_unit_test.py

@@ -13,14 +13,19 @@
 from __future__ import annotations
 
 import tempfile
+import threading
 import xmlrpc.client
 from pathlib import Path
+from typing import TYPE_CHECKING
 
 import numpy as np
 import pytest
 from sigima.objects import create_signal
 
-from datalab.tests import datalab_in_background_context
+from datalab.tests import close_datalab_background, datalab_in_background_context
+
+if TYPE_CHECKING:
+    from datalab.control.proxy import RemoteProxy
 
 
 @pytest.mark.parametrize("mode", ["files", "directory"])
@@ -56,8 +61,41 @@ def test_remote_command_error() -> None:
         assert proxy.get_current_panel() == "signal"
 
 
+def test_remote_command_result() -> None:
+    """The result of a remote command is not returned by the next command"""
+    with datalab_in_background_context() as proxy:
+        assert proxy.call_method("get_current_panel") == "signal"
+        # No object: the result of the previous command must not be returned instead
+        assert proxy.get_current_object_uuid() is None
+
+
+def test_remote_close_during_call() -> None:
+    """Closing DataLab while a remote call is pending does not hang DataLab"""
+    errors: list[Exception] = []
+
+    def close_datalab(proxy: RemoteProxy) -> None:
+        """Close DataLab with a remote call waiting for DataLab to be ready"""
+        try:
+            proxy.call_method("close")
+        except Exception as exc:  # pylint: disable=broad-except
+            errors.append(exc)
+
+    with datalab_in_background_context() as proxy:
+        thread = threading.Thread(target=close_datalab, args=(proxy,), daemon=True)
+        thread.start()
+        thread.join(20.0)
+        hung = thread.is_alive()
+        if hung:
+            # Requesting a hung DataLab to close would hang the test as well
+            close_datalab_background(request_close=False)
+        assert not hung, "DataLab hung while closing during remote call"
+        assert not errors
+
+
 if __name__ == "__main__":
     with tempfile.TemporaryDirectory() as tmpdir:
         for load_mode in ("files", "directory"):
             test_remote_load_at_startup(Path(tmpdir), load_mode)
     test_remote_command_error()
+    test_remote_command_result()
+    test_remote_close_during_call()

doc/release_notes/release_1.04.md

@@ -172,3 +172,5 @@ DataLab now uses stable UUID-based object references across object trees, result
 
 * Fixed files loaded remotely with `load_from_files` or `load_from_directory` being sometimes silently ignored, or still loading after the call had returned, when requested right after DataLab started: requests received while DataLab is starting are now processed as soon as its main window is ready (fixes [Issue #398](https://github.com/DataLab-Platform/DataLab/issues/398))
 * Errors raised by remote commands, such as `calc` with an unknown computation function or `set_object` with an unknown object, are now reported to the client instead of being silently ignored
+* Fixed remote commands sometimes returning the result of the previous command instead of their own: for example, `get_current_object_uuid` could return a panel name instead of `None` when there was no current object
+* Fixed DataLab freezing when closed while a remote command was still running (e.g. when closing the main window with `call_method("close")`)