Skip to content

Commit

Permalink
Prevent parameter retrieval crashes (#978)
Browse files Browse the repository at this point in the history
* fix(params): prevent parameter retrieval crashes in ROS2

- Add check for node's fully qualified name to prevent deadlocks
- Add 2.0s timeout to spin_until_future_complete
- Fix crashes when retrieving parameters via Roslibjs

Fixes #972

* added parameter for timeout

* Add new service to retrieve the different interfaces in the ROS Network (#988)

* Fix rosbridge_websocket symlink

* Don't fail on nodes without parameter services

* Remove added empty line

---------

Co-authored-by: Błażej Sowa <[email protected]>
Co-authored-by: Matthias Rathauscher (LWN) <[email protected]>
Co-authored-by: Lebecque Florian <[email protected]>
  • Loading branch information
4 people authored Jan 25, 2025
1 parent 528b21f commit 0d96ae5
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 9 deletions.
5 changes: 4 additions & 1 deletion rosapi/scripts/rosapi_node
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class Rosapi(Node):
self.declare_parameter("topics_glob", "[*]")
self.declare_parameter("services_glob", "[*]")
self.declare_parameter("params_glob", "[*]")
self.declare_parameter("params_timeout", 5.0)
self.globs = self.get_globs()
self.register_services()

Expand All @@ -88,7 +89,9 @@ class Rosapi(Node):
full_name = self.get_namespace() + self.get_name()
else:
full_name = self.get_namespace() + "/" + self.get_name()
params.init(full_name)

timeout_sec = self.get_parameter("params_timeout").value
params.init(full_name, timeout_sec)

self.create_service(Topics, "~/topics", self.get_topics)
self.create_service(Interfaces, "~/interfaces", self.get_interfaces)
Expand Down
23 changes: 15 additions & 8 deletions rosapi/src/rosapi/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,14 @@
""" Methods to interact with the param server. Values have to be passed
as JSON in order to facilitate dynamically typed SRV messages """

# Constants
DEFAULT_PARAM_TIMEOUT_SEC = 5.0

# Ensure thread safety for setting / getting parameters.
param_server_lock = threading.RLock()
_node = None
_parent_node_name = ""
_timeout_sec = DEFAULT_PARAM_TIMEOUT_SEC

_parameter_type_mapping = [
"",
Expand All @@ -64,12 +68,12 @@
]


def init(parent_node_name):
def init(parent_node_name, timeout_sec=DEFAULT_PARAM_TIMEOUT_SEC):
"""
Initializes params module with a rclpy.node.Node for further use.
This function has to be called before any other for the module to work.
"""
global _node, _parent_node_name
global _node, _parent_node_name, _timeout_sec
# TODO(@jubeira): remove this node; use rosapi node with MultiThreadedExecutor or
# async / await to prevent the service calls from blocking.
parent_node_basename = parent_node_name.split("/")[-1]
Expand All @@ -81,6 +85,10 @@ def init(parent_node_name):
)
_parent_node_name = get_absolute_node_name(parent_node_name)

if not isinstance(timeout_sec, (int, float)) or timeout_sec <= 0:
raise ValueError("Parameter timeout must be a positive number")
_timeout_sec = timeout_sec


def set_param(node_name, name, value, params_glob):
"""Sets a parameter in a given node"""
Expand Down Expand Up @@ -226,21 +234,20 @@ def _get_param_names(node_name):
# This method is called in a service callback; calling a service of the same node
# will cause a deadlock.
global _parent_node_name
if node_name == _parent_node_name:
if node_name == _parent_node_name or node_name == _node.get_fully_qualified_name():
return []

client = _node.create_client(ListParameters, f"{node_name}/list_parameters")

ready = client.wait_for_service(timeout_sec=5.0)
if not ready:
raise RuntimeError("Wait for list_parameters service timed out")
if not client.service_is_ready():
return []

request = ListParameters.Request()
future = client.call_async(request)
if _node.executor:
_node.executor.spin_until_future_complete(future)
_node.executor.spin_until_future_complete(future, timeout_sec=_timeout_sec)
else:
rclpy.spin_until_future_complete(_node, future)
rclpy.spin_until_future_complete(_node, future, timeout_sec=_timeout_sec)
response = future.result()

if response is not None:
Expand Down
2 changes: 2 additions & 0 deletions rosbridge_server/launch/rosbridge_websocket_launch.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
<arg name="topics_glob" default="" />
<arg name="services_glob" default="" />
<arg name="params_glob" default="" />
<arg name="params_timeout" default="5.0" />
<arg name="bson_only_mode" default="false" />

<arg name="respawn" default="false"/>
Expand Down Expand Up @@ -78,5 +79,6 @@
<param name="topics_glob" value="$(var topics_glob)"/>
<param name="services_glob" value="$(var services_glob)"/>
<param name="params_glob" value="$(var params_glob)"/>
<param name="params_timeout" value="$(var params_timeout)"/>
</node>
</launch>

0 comments on commit 0d96ae5

Please sign in to comment.