SteveMacenski / spatio_temporal_voxel_layer

A new voxel layer leveraging modern 3D graphics tools to modernize navigation environmental representations
http://wiki.ros.org/spatio_temporal_voxel_layer
GNU Lesser General Public License v2.1
620 stars 184 forks source link

Lock parent costmap mutex on updateBounds to avoid deadlock when clearing an area #245

Closed corot closed 1 year ago

corot commented 1 year ago

The deadlock sequence is

  1. updateBounds locks _voxel_grid_lock
  2. clear_costmap recovery locks costmap mutex
  3. clear_costmap recovery calls clearArea
  4. clearArea tries to lock _voxel_grid_lock --> locked waiting for updateBounds to release it
  5. updateBounds tries to lock costmap mutex --> locked waiting for clear_costmap recovery to release it

The alternative is to remove the lock at clearArea. I don't think it is needed

Draft PR until we decide the best fix (and clear the TODOs added for info)

To reproduce I use this simple script:

#!/usr/bin/env python

import rospy
import actionlib

from mbf_msgs.msg import RecoveryAction, RecoveryGoal, RecoveryResult

""" 
Continuously call recovery action.
"""

def call_recovery(seq, behavior):
    rospy.loginfo("Calling recovery behavior %s", behavior)
    recovery_goal = RecoveryGoal(concurrency_slot=seq, behavior=behavior)
    recovery_ac.send_goal(recovery_goal)

if __name__ == '__main__':
    rospy.init_node("spam_mbf_servers")

    recovery_ac = actionlib.SimpleActionClient("/move_base_flex/recovery", RecoveryAction)
    recovery_ac.wait_for_server(rospy.Duration(5))
    for i in range(1000):
        call_recovery(i, 'conservative_reset')

Note that I keep increasing concurrency_slot, so MBF calls recovery in parallel. In theory sequential calls should be enough, but I cannot make it fail that way.

SteveMacenski commented 1 year ago

match for ROS 2 please :-)

I also asked @onjen from #246 to review

corot commented 1 year ago

match for ROS 2 please :-)

247

corot commented 1 year ago

@SteveMacenski san, happy new year,,, maybe time for a happy new release? :thinking:

SteveMacenski commented 1 year ago

I did a series of them right before the holidays but STVL was still in a funky state with the deadlock. It’ll be on my next release day Feb 10 unless there’s a rush for sooner. I typically batch release a few dozen packages in one sitting

corot commented 1 year ago

It’ll be on my next release day Feb 10 unless there’s a rush for sooner. I typically batch release a few dozen packages in one sitting

sounds good; thanks!