Use RAII to handle mutexes

include/actionlib/client/simple_action_client.h

@@ -537,9 +537,10 @@ void SimpleActionClient<ActionSpec>::handleTransition(GoalHandleT gh)
       switch (cur_simple_state_.state_) {
         case SimpleGoalState::PENDING:
         case SimpleGoalState::ACTIVE:
-          done_mutex_.lock();
-          setSimpleState(SimpleGoalState::DONE);
-          done_mutex_.unlock();
+          {
+            boost::mutex::scoped_lock lock(done_mutex_);
+            setSimpleState(SimpleGoalState::DONE);
+          }
 
           if (done_cb_) {
             done_cb_(getState(), gh.getResult());

include/actionlib/server/action_server.h

@@ -39,6 +39,7 @@
 
 #include <ros/ros.h>
 #include <boost/thread.hpp>
+#include <boost/thread/reverse_lock.hpp>
 #include <boost/shared_ptr.hpp>
 #include <actionlib_msgs/GoalID.h>
 #include <actionlib_msgs/GoalStatusArray.h>

include/actionlib/server/action_server_base.h

@@ -253,12 +253,10 @@ void ActionServerBase<ActionSpec>::goalCallback(const boost::shared_ptr<const Ac
     GoalHandle gh = GoalHandle(it, this, handle_tracker, guard_);
 
     // make sure that we unlock before calling the users callback
-    lock_.unlock();
+    boost::reverse_lock<boost::recursive_mutex::scoped_lock> unlocker(lock);
 
     // now, we need to create a goal handle and call the user's callback
     goal_callback_(gh);
-
-    lock_.lock();
   }
 }
 
@@ -310,13 +308,10 @@ void ActionServerBase<ActionSpec>::cancelCallback(
       GoalHandle gh(it, this, handle_tracker, guard_);
       if (gh.setCancelRequested()) {
         // make sure that we're unlocked before we call the users callback
-        lock_.unlock();
+        boost::reverse_lock<boost::recursive_mutex::scoped_lock> unlocker(lock);
 
         // call the user's cancel callback on the relevant goal
         cancel_callback_(gh);
-
-        // lock for further modification of the status list
-        lock_.lock();
       }
     }
   }

include/actionlib/server/handle_tracker_deleter_imp.h

@@ -55,10 +55,9 @@ void HandleTrackerDeleter<ActionSpec>::operator()(void *)
     DestructionGuard::ScopedProtector protector(*guard_);
     if (protector.isProtected()) {
       // make sure to lock while we erase status for this goal from the list
-      as_->lock_.lock();
+      boost::recursive_mutex::scoped_lock lock(as_->lock_);
       (*status_it_).handle_destruction_time_ = ros::Time::now();
       // as_->status_list_.erase(status_it_);
-      as_->lock_.unlock();
     }
   }
 }

include/actionlib/server/simple_action_server_imp.h

@@ -385,10 +385,11 @@ void SimpleActionServer<ActionSpec>::executeLoop()
       ROS_FATAL_COND(!execute_callback_,
         "execute_callback_ must exist. This is a bug in SimpleActionServer");
 
-      // Make sure we're not locked when we call execute
-      lock.unlock();
-      execute_callback_(goal);
-      lock.lock();
+      {
+        // Make sure we're not locked when we call execute
+        boost::reverse_lock<boost::recursive_mutex::scoped_lock> unlocker(lock);
+        execute_callback_(goal);
+      }
 
       if (isActive()) {
         ROS_WARN_NAMED("actionlib", "Your executeCallback did not set the goal to a terminal status.\n"