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:
- ROS2 Version:
- Version or commit hash:
- DDS implementation:
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.
During the review of #3297, I've discovered that in the
Costmap2DROSthere is no guarantee thatplugins->intialize()will be called before their actual working methods:plugins->updateBounds()andplugins->updateCosts().The current situation is:
Costmap2DROS::on_activatecreates and startsmap_update_thread_with main costmap updating loop and at the same time in parallel callsstart()routine which contains plugin activation procedures calls:(*plugin)->activate();Typically, during local experiments,
plugin->activate()is being called first, beforeplugin->updateBounds(), but it is not guaranteed. For example, one could be added some sleep instart()before plugin activation will be called:and
plugin->updateCosts/Boundsroutines 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 onupdateCosts(). This will cause null pointer dereference, if callupdateCosts()beforeactivate()stage.Bug report
Required Info:
Steps to reproduce issue
Costmap2DROS::start()routine right before callingplugin->activate()updateCosts()orupdateBounds()Implementation considerations
Here is already exists
stopped_variable, which sets tofalseafter plugins activation stage will be completed.In the
Costmap2DROS::updateMap()we could calllayered_costmap_->updateMap();only ifstopped_isfalse.