diff --git a/ros2cli/ros2cli/node/daemon.py b/ros2cli/ros2cli/node/daemon.py index 7dbca380e..38ef4167d 100644 --- a/ros2cli/ros2cli/node/daemon.py +++ b/ros2cli/ros2cli/node/daemon.py @@ -29,6 +29,12 @@ from ros2cli.xmlrpc.client import ServerProxy +# The daemon server polls for shutdown every 0.2 seconds. Allow several such +# intervals for Windows process teardown without letting a foreign port owner +# consume the caller's full (or indefinite) daemon startup timeout. +_DAEMON_SOCKET_RELEASE_GRACE_PERIOD = 1.0 + + class DaemonNode: def __init__(self, args): @@ -90,6 +96,38 @@ def _is_daemon_address_free(): raise +def _make_xmlrpc_server_when_available(args, timeout): + """Acquire the daemon XML-RPC server socket, waiting out a Windows shutdown tail.""" + try: + return daemon.make_xmlrpc_server() + except socket.error as e: + if e.errno != errno.EADDRINUSE: + raise + + # A live daemon owns the address legitimately. The shutdown-tail behavior + # addressed here is Windows-specific; on other platforms preserve the + # previous immediate EADDRINUSE result. No requested timeout also remains + # non-blocking. + if is_daemon_running(args) or timeout is None or os.name != 'nt': + return None + + # A Windows daemon can stop serving before its process releases the socket. + # Use a short bounded grace period rather than the full spawn timeout: a + # foreign process can own this fixed port too, and must not make startup + # block indefinitely when the caller requested an indefinite daemon wait. + grace_period = _DAEMON_SOCKET_RELEASE_GRACE_PERIOD + if timeout > 0: + grace_period = min(timeout, grace_period) + if not wait_for(_is_daemon_address_free, grace_period): + return None + try: + return daemon.make_xmlrpc_server() + except socket.error as e: + if e.errno == errno.EADDRINUSE: + return None + raise + + def shutdown_daemon(args, timeout=None): """ Shut down daemon node if it's running. @@ -141,19 +179,14 @@ def spawn_daemon(args, timeout=None, debug=False, inactivity_timeout=2 * 60 * 60 disables the timeout, so the daemon runs until explicitly stopped. :return: `True` if the daemon was spawned, - `False` if it was already running. + `False` if it was already running or its address remained busy. :raises: if it fails to spawn the daemon. """ # Acquire socket by instantiating XMLRPC server. - try: - server = daemon.make_xmlrpc_server() - server.socket.set_inheritable(True) - except socket.error as e: - if e.errno == errno.EADDRINUSE: - # Failed to acquire socket - # Daemon already running - return False - raise + server = _make_xmlrpc_server_when_available(args, timeout) + if server is None: + return False + server.socket.set_inheritable(True) # During tab completion on the ros2 tooling, we can get here and attempt to spawn a daemon. # In that scenario, there may be open file descriptors that can prevent us from successfully diff --git a/ros2cli/test/test_daemon_socket_acquisition.py b/ros2cli/test/test_daemon_socket_acquisition.py new file mode 100644 index 000000000..7c031ab7d --- /dev/null +++ b/ros2cli/test/test_daemon_socket_acquisition.py @@ -0,0 +1,107 @@ +# Copyright 2026 Sylvester Kaczmarek +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import errno +import socket +from unittest.mock import Mock +from unittest.mock import patch + +import ros2cli.node.daemon as daemon_node + + +def _address_in_use_error(): + return socket.error(errno.EADDRINUSE, 'address already in use') + + +def test_busy_address_with_running_daemon_is_not_retried(): + with patch.object( + daemon_node.daemon, + 'make_xmlrpc_server', + side_effect=_address_in_use_error(), + ), patch.object( + daemon_node, 'is_daemon_running', return_value=True + ), patch.object(daemon_node, 'wait_for') as wait_for: + server = daemon_node._make_xmlrpc_server_when_available([], 5.0) + + assert server is None + wait_for.assert_not_called() + + +def test_busy_windows_shutdown_tail_is_retried_after_address_becomes_free(): + expected_server = Mock() + with patch.object( + daemon_node.daemon, + 'make_xmlrpc_server', + side_effect=[_address_in_use_error(), expected_server], + ), patch.object( + daemon_node, 'is_daemon_running', return_value=False + ), patch.object(daemon_node.os, 'name', 'nt'), patch.object( + daemon_node, 'wait_for', return_value=True + ) as wait_for: + server = daemon_node._make_xmlrpc_server_when_available([], 5.0) + + assert server is expected_server + wait_for.assert_called_once_with( + daemon_node._is_daemon_address_free, + daemon_node._DAEMON_SOCKET_RELEASE_GRACE_PERIOD, + ) + + +def test_foreign_windows_port_owner_cannot_consume_full_spawn_timeout(): + with patch.object( + daemon_node.daemon, + 'make_xmlrpc_server', + side_effect=_address_in_use_error(), + ), patch.object( + daemon_node, 'is_daemon_running', return_value=False + ), patch.object(daemon_node.os, 'name', 'nt'), patch.object( + daemon_node, 'wait_for', return_value=False + ) as wait_for: + server = daemon_node._make_xmlrpc_server_when_available([], -1.0) + + assert server is None + wait_for.assert_called_once_with( + daemon_node._is_daemon_address_free, + daemon_node._DAEMON_SOCKET_RELEASE_GRACE_PERIOD, + ) + + +def test_busy_non_windows_address_is_not_retried(): + with patch.object( + daemon_node.daemon, + 'make_xmlrpc_server', + side_effect=_address_in_use_error(), + ), patch.object( + daemon_node, 'is_daemon_running', return_value=False + ), patch.object(daemon_node.os, 'name', 'posix'), patch.object( + daemon_node, 'wait_for' + ) as wait_for: + server = daemon_node._make_xmlrpc_server_when_available([], 5.0) + + assert server is None + wait_for.assert_not_called() + + +def test_busy_address_without_timeout_preserves_non_blocking_behavior(): + with patch.object( + daemon_node.daemon, + 'make_xmlrpc_server', + side_effect=_address_in_use_error(), + ), patch.object( + daemon_node, 'is_daemon_running', return_value=False + ), patch.object(daemon_node, 'wait_for') as wait_for: + server = daemon_node._make_xmlrpc_server_when_available([], None) + + assert server is None + wait_for.assert_not_called()