Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 31 additions & 10 deletions ros2cli/ros2cli/node/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,32 @@ def _is_daemon_address_free():
raise


def _make_xmlrpc_server_when_available(args, timeout):
"""Acquire the daemon XML-RPC server socket, waiting out a shutdown tail if requested."""
try:
return daemon.make_xmlrpc_server()
except socket.error as e:
if e.errno != errno.EADDRINUSE:
raise

# A live daemon owns the address legitimately. If no wait was requested,
# preserve the existing non-blocking behavior for any occupied address.
if is_daemon_running(args) or timeout is None:
return None

# On Windows a daemon can stop serving before its process releases the
# listening socket. Give that shutdown tail time to finish, then retry the
# bind. Another process may win the race, in which case it owns the socket.
if not wait_for(_is_daemon_address_free, timeout):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

full-timeout stall (and possible infinite wait) when a foreign process holds the daemon port?
the daemon address is a fixed loopback port derived from ROS_DOMAIN_ID; any unrelated process can hold it.
if that happens, is_daemon_running() is false, and wait_for(_is_daemon_address_free, timeout) polls a predicate that can never become true.
the CLI blocks for the entire spawn timeout, then returns the same false the pre-PR code returned immediately though. this could be worse for UX, the module's documented timeout convention

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.
Expand Down Expand Up @@ -141,19 +167,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
Expand Down
67 changes: 67 additions & 0 deletions ros2cli/test/test_daemon_socket_acquisition.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# 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_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, '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, 5.0)


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()