rst-tu-dortmund / teb_local_planner

An optimal trajectory planner considering distinctive topologies for mobile robots based on Timed-Elastic-Bands (ROS Package)
http://wiki.ros.org/teb_local_planner
BSD 3-Clause "New" or "Revised" License
984 stars 545 forks source link

fix segfault at robot-model update (#255 revisited) #393

Closed corot closed 1 year ago

corot commented 1 year ago

Resurrecting #255:

We found exactly the same issue #245, reproduced in the same way, and I can confirm that @dorezyuk's PR #255 fixes it

I open a new PR because:

About @amakarow's comment, I think the code becomes cleaner now (if nothing else, we have almost 4 to 1 deletions!)

@dorezyuk, can u explain why the lock is needed? Thanks, and thanks for the fix!

siferati commented 1 year ago

We need the lock because otherwise you're modifying a field of the config object outside the protection of the mutex. The other fields are protected by the mutex inside TebConfig::reconfigure, but robot_model is not since you set it outside in TebLocalPlannerROS::reconfigureCB.

It's the same problem we had with DWA a while ago

corot commented 1 year ago

We need the lock because otherwise you're modifying a field of the config object outside the protection of the mutex. The other fields are protected by the mutex inside TebConfig::reconfigure, but robot_model is not since you set it outside in TebLocalPlannerROS::reconfigureCB.

It's the same problem we had with DWA a while ago

added

zst1406217 commented 1 year ago

This is the exact problem I have in my research. When I reconfigure the parameters, the segfault occurs occasionally, but not from the beginning, this has confused me a long time. I prove this is helpful!

corot commented 1 year ago

This is the exact problem I have in my research. When I reconfigure the parameters, the segfault occurs occasionally, but not from the beginning, this has confused me a long time. I prove this is helpful!

Thanks for reporting. Merged