From 994913cc9f8bc3503832eb3defe5c0deae4aef33 Mon Sep 17 00:00:00 2001 From: Pierre Raybaut <1311787+PierreRaybaut@users.noreply.github.com> Date: Tue, 20 Oct 2026 10:34:51 +0200 Subject: [PATCH 1/4] fix(remote): process XML-RPC requests once DataLab is ready (#398) The XML-RPC server was started before the main window was built, so requests could be processed while the window was still being set up: whenever setup code pumped the event loop, a queued command ran on a partially built window. load_from_files and load_from_directory then failed because the History panel did not exist yet, and calls could return before their command was done because readiness signaled by setup code released them. - The server binds its port at startup, but serves requests only once the main window is set up (_after_setup): clients connecting earlier wait instead of being refused. A server still waiting can be shut down. - Remote commands run through run_remote_command, which stores the exception raised by the command before signaling readiness, so that the error is reported to the client instead of being silently ignored. The stored exception is reset at the start of each request. - add_object no longer works around a missing History panel, which existed only because of this startup race. Assisted-by: Claude Opus 5.5 --- datalab/control/remote.py | 63 +++++++++++++++++++++++++-------------- datalab/gui/main.py | 32 +++++++++++++++----- 2 files changed, 65 insertions(+), 30 deletions(-) diff --git a/datalab/control/remote.py b/datalab/control/remote.py index 35ceae36b..36c2c04af 100644 --- a/datalab/control/remote.py +++ b/datalab/control/remote.py @@ -50,6 +50,7 @@ def method_wrapper(*args, **kwargs): """Decorator wrapper function""" self: RemoteServer = args[0] # extracting 'self' from method arguments self.is_ready = False + self.exception = None output = func(*args, **kwargs) while not self.is_ready: QC.QCoreApplication.processEvents() @@ -116,33 +117,37 @@ def __init__(self, win: DLMainWindow) -> None: self.win = win self.result = None self.exception = None + self.__window_ready = threading.Event() win.SIG_READY.connect(self.datalab_is_ready) win.SIG_CLOSING.connect(self.shutdown_server) self.SIG_CLOSE_APP.connect(win.close) self.SIG_RAISE_WINDOW.connect(win.raise_window) - self.SIG_ADD_OBJECT.connect(win.add_object) - self.SIG_SET_OBJECT.connect(win.set_object) - self.SIG_ADD_GROUP.connect(win.add_group) - self.SIG_LOAD_FROM_FILES.connect(win.load_from_files) - self.SIG_LOAD_FROM_DIRECTORY.connect(win.load_from_directory) - self.SIG_SELECT_OBJECTS.connect(win.select_objects) - self.SIG_SELECT_GROUPS.connect(win.select_groups) - self.SIG_SELECT_ALL_GROUPS.connect(lambda panel: win.select_groups(None, panel)) - self.SIG_DELETE_METADATA.connect(win.delete_metadata) - self.SIG_SWITCH_TO_PANEL.connect(win.set_current_panel) - self.SIG_TOGGLE_AUTO_REFRESH.connect(win.toggle_auto_refresh) - self.SIG_TOGGLE_SHOW_TITLES.connect(win.toggle_show_titles) - self.SIG_RESET_ALL.connect(win.reset_all) - self.SIG_REMOVE_OBJECT.connect(win.remove_object) - self.SIG_SAVE_TO_H5.connect(win.save_to_h5_file) - self.SIG_OPEN_H5.connect(win.open_h5_files) - self.SIG_IMPORT_H5.connect(win.import_h5_file) - self.SIG_LOAD_H5_WORKSPACE.connect(win.load_h5_workspace) - self.SIG_SAVE_H5_WORKSPACE.connect(win.save_h5_workspace) - self.SIG_CALC.connect(win.calc) - self.SIG_RUN_MACRO.connect(win.run_macro) - self.SIG_STOP_MACRO.connect(win.stop_macro) - self.SIG_IMPORT_MACRO_FROM_FILE.connect(win.import_macro_from_file) + for signal, command in ( + (self.SIG_ADD_OBJECT, win.add_object), + (self.SIG_SET_OBJECT, win.set_object), + (self.SIG_ADD_GROUP, win.add_group), + (self.SIG_LOAD_FROM_FILES, win.load_from_files), + (self.SIG_LOAD_FROM_DIRECTORY, win.load_from_directory), + (self.SIG_SELECT_OBJECTS, win.select_objects), + (self.SIG_SELECT_GROUPS, win.select_groups), + (self.SIG_SELECT_ALL_GROUPS, functools.partial(win.select_groups, None)), + (self.SIG_DELETE_METADATA, win.delete_metadata), + (self.SIG_SWITCH_TO_PANEL, win.set_current_panel), + (self.SIG_TOGGLE_AUTO_REFRESH, win.toggle_auto_refresh), + (self.SIG_TOGGLE_SHOW_TITLES, win.toggle_show_titles), + (self.SIG_RESET_ALL, win.reset_all), + (self.SIG_REMOVE_OBJECT, win.remove_object), + (self.SIG_SAVE_TO_H5, win.save_to_h5_file), + (self.SIG_OPEN_H5, win.open_h5_files), + (self.SIG_IMPORT_H5, win.import_h5_file), + (self.SIG_LOAD_H5_WORKSPACE, win.load_h5_workspace), + (self.SIG_SAVE_H5_WORKSPACE, win.save_h5_workspace), + (self.SIG_CALC, win.calc), + (self.SIG_RUN_MACRO, win.run_macro), + (self.SIG_STOP_MACRO, win.stop_macro), + (self.SIG_IMPORT_MACRO_FROM_FILE, win.import_macro_from_file), + ): + signal.connect(functools.partial(win.run_remote_command, command)) self.SIG_CALL_METHOD.connect(win.call_method_slot) def serve(self) -> None: @@ -157,11 +162,23 @@ def serve(self) -> None: self.register_functions(server) self.port = server.server_address[1] self.notify_port(self.port) + # Clients may already connect: their requests wait for the main window + self.__window_ready.wait() with execenv.context(xmlrpcport=self.port): server.serve_forever() + def notify_window_ready(self) -> None: + """Notify that the main window is fully set up. + + Requests are processed from then on: processing them earlier would run + commands on a partially built main window. + """ + self.__window_ready.set() + def shutdown_server(self) -> None: """Shutdown server""" + # A server still waiting for the main window must be able to stop + self.__window_ready.set() if self.server is not None: self.server.shutdown() self.server = None diff --git a/datalab/gui/main.py b/datalab/gui/main.py index b7f40e79c..a92da45f5 100644 --- a/datalab/gui/main.py +++ b/datalab/gui/main.py @@ -106,7 +106,7 @@ from datalab.widgets.expandabletext import refresh_subdued_colors if TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import Callable, Iterator from typing import Any, Literal from datalab.gui.historysession_ops import SessionBehavior @@ -221,7 +221,8 @@ def _before_setup(self, console: bool) -> None: self.h5inputoutput = H5InputOutput(self) self.capsuleio = CapsuleInputOutput(self) - # Starting XML-RPC server thread + # Starting XML-RPC server thread: requests are processed once the main + # window is fully set up (see `_after_setup`) self.remote_server = RemoteServer(self) if Conf.rpc_server_enabled.get(): self.remote_server.SIG_SERVER_PORT.connect(self.xmlrpc_server_started) @@ -230,6 +231,11 @@ def _before_setup(self, console: bool) -> None: self.__register_plugins() self.webapi_actions = WebApiActions(self) + def _after_setup(self, console: bool) -> None: + """Process remote requests, the main window being fully set up.""" + super()._after_setup(console) + self.remote_server.notify_window_ready() + # ------API related to XML-RPC remote control @staticmethod def xmlrpc_server_started(port): @@ -667,6 +673,23 @@ def call_method_slot( self.remote_server.result = None self.remote_server.exception = exc + @remote_controlled + def run_remote_command(self, command: Callable[..., object], *args) -> None: + """Slot to run a command requested by a remote client in GUI thread. + + The exception raised by the command, if any, is stored in RemoteServer to + be re-raised to the client. Being remote controlled, this slot signals that + DataLab is ready only once the command is done and its exception stored. + + Args: + command: Main window method running the command + args: Command arguments + """ + try: + command(*args) + except Exception as exc: # pylint: disable=broad-except + self.remote_server.exception = exc + @remote_controlled def get_object_shapes( self, @@ -2518,12 +2541,7 @@ def add_object( panel_str = "image" else: raise TypeError(f"Unsupported object type {type(obj)}") - # A remote call can reach the main window before the History panel dock - # exists, since the XML-RPC server is started before the UI is built. historypanel = self.historypanel - if historypanel is None: - panel.add_object(obj, group_id, set_current) - return True historypanel.maybe_start_session_for_input(behavior=new_session_behavior) panel.add_object(obj, group_id, set_current) # Record a creation entry so objects added programmatically (plugins, From c86778e8a87eb83da33696a706ad736e5ae75fd0 Mon Sep 17 00:00:00 2001 From: Pierre Raybaut <1311787+PierreRaybaut@users.noreply.github.com> Date: Tue, 20 Oct 2026 10:35:12 +0200 Subject: [PATCH 2/4] test(remote): remote requests at startup and error reporting (#398) Files loaded with load_from_files and load_from_directory right after connecting to a background DataLab must be loaded when the call returns, without polling. Errors raised by remote commands (calc with an unknown function, set_object with an unknown object) must be reported to the client, and not again by the next command. Assisted-by: Claude Opus 5.5 --- .../control/remote_requests_unit_test.py | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 datalab/tests/features/control/remote_requests_unit_test.py diff --git a/datalab/tests/features/control/remote_requests_unit_test.py b/datalab/tests/features/control/remote_requests_unit_test.py new file mode 100644 index 000000000..1c47f970a --- /dev/null +++ b/datalab/tests/features/control/remote_requests_unit_test.py @@ -0,0 +1,63 @@ +# Copyright (c) DataLab Platform Developers, BSD 3-Clause license, see LICENSE file. + +""" +Remote requests synchronization tests + +A remote command must be done when its XML-RPC call returns, or report an error, +including when it is sent as soon as DataLab accepts connections, i.e. while the +main window may still be set up (see DataLab Issue #398). +""" + +# guitest: show + +from __future__ import annotations + +import tempfile +import xmlrpc.client +from pathlib import Path + +import numpy as np +import pytest +from sigima.objects import create_signal + +from datalab.tests import datalab_in_background_context + + +@pytest.mark.parametrize("mode", ["files", "directory"]) +def test_remote_load_at_startup(tmp_path: Path, mode: str) -> None: + """Files loaded remotely right after connecting are loaded when the call returns + + Args: + tmp_path: Temporary directory + mode: "files" to call ``load_from_files``, "directory" to call + ``load_from_directory`` + """ + filename = tmp_path / "image.npy" + np.save(filename, np.arange(6, dtype=np.uint16).reshape(2, 3)) + with datalab_in_background_context() as proxy: + proxy.set_current_panel("image") + if mode == "files": + proxy.load_from_files([str(filename)]) + else: + proxy.load_from_directory(str(tmp_path)) + assert len(proxy.get_object_uuids("image")) == 1 + + +def test_remote_command_error() -> None: + """Errors raised by remote commands are reported to the client""" + with datalab_in_background_context() as proxy: + with pytest.raises(xmlrpc.client.Fault, match="Unknown computation function"): + proxy.calc("unknown_function") + unknown = create_signal("Unknown signal", np.arange(3.0), np.zeros(3)) + with pytest.raises(xmlrpc.client.Fault, match="KeyError"): + proxy.set_object(unknown) + # The error must not be reported again by the next command + proxy.set_current_panel("signal") + assert proxy.get_current_panel() == "signal" + + +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() From 63235f998545e838b4c961ce9f64c38b2ee3455f Mon Sep 17 00:00:00 2001 From: Pierre Raybaut <1311787+PierreRaybaut@users.noreply.github.com> Date: Tue, 20 Oct 2026 10:35:23 +0200 Subject: [PATCH 3/4] docs: release notes for remote control fixes (#398) Assisted-by: Claude Opus 5.5 --- doc/release_notes/release_1.04.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/release_notes/release_1.04.md b/doc/release_notes/release_1.04.md index 718d4b8e4..52990187e 100644 --- a/doc/release_notes/release_1.04.md +++ b/doc/release_notes/release_1.04.md @@ -167,3 +167,8 @@ DataLab now uses stable UUID-based object references across object trees, result **Signal creation:** * The blackbody signal (**Create > Blackbody (Planck's law)**) now has axis titles and units (wavelength in m, spectral radiance in W·sr⁻¹·m⁻³), which were missing (fixes [Issue #400](https://github.com/DataLab-Platform/DataLab/issues/400)) + +**Remote control:** + +* 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 From 42583a18bc82eeeeade8b3da13020009e828ad5e Mon Sep 17 00:00:00 2001 From: Pierre Raybaut <1311787+PierreRaybaut@users.noreply.github.com> Date: Tue, 20 Oct 2026 12:15:51 +0200 Subject: [PATCH 4/4] 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 --- datalab/control/remote.py | 5 +++ .../control/remote_requests_unit_test.py | 40 ++++++++++++++++++- doc/release_notes/release_1.04.md | 2 + 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/datalab/control/remote.py b/datalab/control/remote.py index 36c2c04af..40275fb69 100644 --- a/datalab/control/remote.py +++ b/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 diff --git a/datalab/tests/features/control/remote_requests_unit_test.py b/datalab/tests/features/control/remote_requests_unit_test.py index 1c47f970a..01065397e 100644 --- a/datalab/tests/features/control/remote_requests_unit_test.py +++ b/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() diff --git a/doc/release_notes/release_1.04.md b/doc/release_notes/release_1.04.md index 52990187e..6c627077c 100644 --- a/doc/release_notes/release_1.04.md +++ b/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")`)