User misconfiguration of parameters may cause instantaneous crashs

[GoesM]

Bug report

Required Info:

• Operating System:
  • Ubuntu 22.04
• ROS2 Version:
  • humble
• Version or commit hash:
  • 1134569
• DDS implementation:
  • default

Steps to reproduce issue

launch the navigation2 within defaulted tb3_simulation_launch.py as following commands

source install/setup.bash
source /opt/ros/humble/setup.bash
export TURTLEBOT3_MODEL=waffle
export GAZEBO_MODEL_PATH=$GAZEBO_MODEL_PATH:/opt/ros/humble/share/turtlebot3_gazebo/models
ros2 launch nav2_bringup tb3_simulation_launch.py params_file:=my_nav2_params.yaml

there's only one difference between my_nav2_params.yaml and defaulted nav2_params.yaml, (we wrongly set the height of local_costmap as -20), as following:

...
...
local_costmap:
  local_costmap:
    ros__parameters:
      ...
      ...
      height: -20
...
...

and then , try to set an /initial_pose in rviz by 2D_estimate_pose or send a msg into /initial_pose in another terminal

Expected behavior

• process should not crash / at least, it should not make the user's computer unable to continue working
• the memory usage should not be too high

Actual behavior

• process crashed when launching
  specifically, during the startup process of nav2, there may be computer lag, terminal crashes, or even a blue screen
  

• abnormal memory usage
  we found that nav2_controller process occupies a very large amount of memory (up to 10G, sometimes even exceeding 20G, which is very abnormal), as following:
  
  here's a normal memory usage when we use the defaulted nav2_params.yaml, which is for comparision.
  
  the max memory usage of whole nav2 is below 1G.

[SteveMacenski]

Thanks for the report - in general its not planned for us to protect users from themselves with every parameter if they choose something impossible. We try where we can, but the codebase would balloon to 2x the size for checking every single parameter for every way that it could be misinterpreted.

I don't disagree with you that you could make the system crash on initialization if you set the width/height to a negative value. I put that in the category of "fair" since its not a crash mid-execution, non-deterministically, or during operations. That is a crash you would see immediately when you tried to bring the system up, every time, that could be debugged before proceeding. I take crashes very seriously, but for parameter misconfiguration of this nature, its not realistic today that we're going to protect users from making errors on every one of a thousand parameters.

However, that's not an unreasonable ask, so I'm going to rename the ticket in line with that and it'll stay in the tracker so that perhaps in the future when we're better resourced we can potentially have some interns do this, but no promises / commitments.

[SteveMacenski]

@GoesM perhaps your extra memory problems are related to your misconfigurations 😉

Ask on Robotics stack exchange with your config file changes and I'll follow up there on that topic. I don't want tickets to become > 1 topic, so I don't want to pollute this thread with that discussion. I hope that makes sense

[GoesM]

I've forwarded this issue to the Robotics stack exchange stack: question 106008.
Thanks for your feedback. 🙂

[Ryanf55]

Keep an eye on this thread for potential solutions to add parameter validation without ballooning the code.
https://discourse.ros.org/t/simplifying-how-to-declare-parameters-in-ros-2/33272

[SteveMacenski]

It is already possible with parameter descriptions. It is more a matter of time / inclination to set bounds on all N params in the stack (and make sure the dynamic parameter callbacks are respecting them too). The blockage isn’t the tech, it’s the value proposition of time.

You should see the amount of code is required per-parameter in this case. Certainly some can be abstracted into Utils, but still.

[SteveMacenski]

Another example #4157

& impl note: #4157 (comment)

[GoesM]

add tickets:

• #4062
• #4335
• #4336
• #4337
• #4338
• #4339