Skip to content

Commit 389022e

Browse files
MehsiasbjsowaMatthias Rathauscher (LWN)FlorianLebecque
committed
Prevent parameter retrieval crashes (backport #978)
* 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 <bsowa123@gmail.com> Co-authored-by: Matthias Rathauscher (LWN) <matthias.rathauscher@liebherr.com> Co-authored-by: Lebecque Florian <Florianlebecque@hotmail.com>
1 parent 8cd4208 commit 389022e

File tree

3 files changed

+22
-9
lines changed

3 files changed

+22
-9
lines changed

rosapi/scripts/rosapi_node

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ class Rosapi(Node):
7878
self.declare_parameter("topics_glob", "[*]")
7979
self.declare_parameter("services_glob", "[*]")
8080
self.declare_parameter("params_glob", "[*]")
81+
self.declare_parameter("params_timeout", 5.0)
8182
self.globs = self.get_globs()
8283
self.register_services()
8384

@@ -88,7 +89,10 @@ class Rosapi(Node):
8889
full_name = self.get_namespace() + self.get_name()
8990
else:
9091
full_name = self.get_namespace() + "/" + self.get_name()
91-
params.init(full_name)
92+
93+
timeout_sec = self.get_parameter("params_timeout").value
94+
params.init(full_name, timeout_sec)
95+
9296
self.create_service(Topics, "/rosapi/topics", self.get_topics)
9397
self.create_service(Interfaces, "/rosapi/interfaces", self.get_interfaces)
9498
self.create_service(TopicsForType, "/rosapi/topics_for_type", self.get_topics_for_type)

rosapi/src/rosapi/params.py

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,14 @@
4444
""" Methods to interact with the param server. Values have to be passed
4545
as JSON in order to facilitate dynamically typed SRV messages """
4646

47+
# Constants
48+
DEFAULT_PARAM_TIMEOUT_SEC = 5.0
49+
4750
# Ensure thread safety for setting / getting parameters.
4851
param_server_lock = threading.RLock()
4952
_node = None
5053
_parent_node_name = ""
54+
_timeout_sec = DEFAULT_PARAM_TIMEOUT_SEC
5155

5256
_parameter_type_mapping = [
5357
"",
@@ -63,12 +67,12 @@
6367
]
6468

6569

66-
def init(parent_node_name):
70+
def init(parent_node_name, timeout_sec=DEFAULT_PARAM_TIMEOUT_SEC):
6771
"""
6872
Initializes params module with a rclpy.node.Node for further use.
6973
This function has to be called before any other for the module to work.
7074
"""
71-
global _node, _parent_node_name
75+
global _node, _parent_node_name, _timeout_sec
7276
# TODO(@jubeira): remove this node; use rosapi node with MultiThreadedExecutor or
7377
# async / await to prevent the service calls from blocking.
7478
parent_node_basename = parent_node_name.split("/")[-1]
@@ -80,6 +84,10 @@ def init(parent_node_name):
8084
)
8185
_parent_node_name = get_absolute_node_name(parent_node_name)
8286

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

8492
def set_param(node_name, name, value, params_glob):
8593
"""Sets a parameter in a given node"""
@@ -225,21 +233,20 @@ def _get_param_names(node_name):
225233
# This method is called in a service callback; calling a service of the same node
226234
# will cause a deadlock.
227235
global _parent_node_name
228-
if node_name == _parent_node_name:
236+
if node_name == _parent_node_name or node_name == _node.get_fully_qualified_name():
229237
return []
230238

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

233-
ready = client.wait_for_service(timeout_sec=5.0)
234-
if not ready:
235-
raise RuntimeError("Wait for list_parameters service timed out")
241+
if not client.service_is_ready():
242+
return []
236243

237244
request = ListParameters.Request()
238245
future = client.call_async(request)
239246
if _node.executor:
240-
_node.executor.spin_until_future_complete(future)
247+
_node.executor.spin_until_future_complete(future, timeout_sec=_timeout_sec)
241248
else:
242-
rclpy.spin_until_future_complete(_node, future)
249+
rclpy.spin_until_future_complete(_node, future, timeout_sec=_timeout_sec)
243250
response = future.result()
244251

245252
if response is not None:

rosbridge_server/launch/rosbridge_websocket_launch.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
<arg name="topics_glob" default="" />
2121
<arg name="services_glob" default="" />
2222
<arg name="params_glob" default="" />
23+
<arg name="params_timeout" default="5.0" />
2324
<arg name="bson_only_mode" default="false" />
2425

2526
<arg unless="$(var bson_only_mode)" name="binary_encoder" default="default"/>
@@ -70,5 +71,6 @@
7071
<param name="topics_glob" value="$(var topics_glob)"/>
7172
<param name="services_glob" value="$(var services_glob)"/>
7273
<param name="params_glob" value="$(var params_glob)"/>
74+
<param name="params_timeout" value="$(var params_timeout)"/>
7375
</node>
7476
</launch>

0 commit comments

Comments
 (0)