-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Planner] possible use-after-free bug in nav2_costmap_2d #2508
Comments
How? Please show in the code where you're talking about. You've provided alot of GDB output, but its unrealistic for me to try to fish through that if I don't understand what your problem is. If you have a solution, please submit a PR. |
Thanks for your reply! Here are my further analyses. It seems a real bug related to multi-threading hard to be detected. If that happens, robot will be out of control. layered_costmap_ is a global value that all plugins have.
incomingMap() can be called at any time after the plugin of static_layer have done onInitialize().
thread two: iterate vector
Those codes above show that voxel_grid_.resize() can be called concurrently.
**Depends on the order of execution in multithreading, it could be use-after-free or double-free. ** |
I have to admit, these seem like incredibly unlikely things to occur, but happy to have fixes! First merged, second I'm thinking about for a little bit but likely to also be merged |
Both merged! I think that closes this ticket out, feel free to comment back if I am incorrect |
I'm experiencing a similar problem. The planner server crashes when a new map arrives while adding a filter to I found two reasons for that:
The code I’m talking about is in nav2_costmap_2d/src/costmap_2d_ros.cpp in
(the same code exists for plugins, so when I’m talking about a filter I also mean a plugin) As already noted by @easylyou ‘layered_costmap_ is a global value that all plugins have’. This variable is set by the So, if multiple static layers exist, and a map is received while My suggested fix would be to change the order of I’m not sure if there is a better way to fix this issue other than this suggestion:
I’m happy to provide a pull request if there is no better solution for the issue. |
Seems good to me! Overall initialization is not something runtime at steady-state, so I'm OK with a little bit of mutex contention if its not at the risk of running at navigation-time |
Bug report
Required Info:
Steps to reproduce issue
I had done some tests. And it can be reproduced.
According to the log of a real launch, it is possible that a resizing costmap of staticLayer happens before voxel_layer complete the initializing.
Expected behavior
Process should not crash.
Actual behavior
The text was updated successfully, but these errors were encountered: