Skip to content

Costmap2D plugins: initialize() could be called after updateBounds()/updateCosts() #3299

@AlexeyMerzlyakov

Description

@AlexeyMerzlyakov

During the review of #3297, I've discovered that in the Costmap2DROS there is no guarantee that plugins->intialize() will be called before their actual working methods: plugins->updateBounds() and plugins->updateCosts().

The current situation is:
Costmap2DROS::on_activate creates and starts map_update_thread_ with main costmap updating loop and at the same time in parallel calls start() routine which contains plugin activation procedures calls: (*plugin)->activate();
Typically, during local experiments, plugin->activate() is being called first, before plugin->updateBounds(), but it is not guaranteed. For example, one could be added some sleep in start() before plugin activation will be called:

diff --git a/nav2_costmap_2d/src/costmap_2d_ros.cpp b/nav2_costmap_2d/src/costmap_2d_ros.cpp
index 50e74c51..52d0f76e 100644
--- a/nav2_costmap_2d/src/costmap_2d_ros.cpp
+++ b/nav2_costmap_2d/src/costmap_2d_ros.cpp
@@ -503,6 +503,11 @@ Costmap2DROS::start()
   std::vector<std::shared_ptr<Layer>> * plugins = layered_costmap_->getPlugins();
   std::vector<std::shared_ptr<Layer>> * filters = layered_costmap_->getFilters();
 
+  RCLCPP_INFO(get_logger(), "Before sleep in start()");
+  rclcpp::Rate rs(0.5);
+  rs.sleep();
+  RCLCPP_INFO(get_logger(), "After sleep in start()");
+
   // check if we're stopped or just paused
   if (stopped_) {
     // if we're stopped we need to re-subscribe to topics

and plugin->updateCosts/Bounds routines will be called first.

For the testcase, I could write a new layer/plugin that makes some shared object inside at the activate() stage, and then uses it on updateCosts(). This will cause null pointer dereference, if call updateCosts() before activate() stage.

Bug report

Required Info:

  • Operating System:
    • Ubuntu 20.04
  • ROS2 Version:
    • ROS2 rolling
  • Version or commit hash:
  • DDS implementation:
    • any

Steps to reproduce issue

  • Insert some delay into Costmap2DROS::start() routine right before calling plugin->activate()
  • Write a plugin that makes some shared objects at activation stage and then uses them at updateCosts() or updateBounds()
  • Get SIGSEGV crash

Implementation considerations

Here is already exists stopped_ variable, which sets to false after plugins activation stage will be completed.
In the Costmap2DROS::updateMap() we could call layered_costmap_->updateMap(); only if stopped_ is false.

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions