Race bug in Actionlib Client where Result from Server can be missed

[pbeeson]

There is a bug in the Actionlib client Goal Manager, where the result from the server can be missed, causing the client application's call to waitForResult() to fail.

A simple patch, which just moves an if-block a few lines away, has been provided in PR #64

I've opened this issue as a way to discuss this or future similar issues, and to provide a way to deterministically reproduce the bug using any Actionlib client/server pair where the server finishes and calls setSucceeded() in under 5 seconds from receiving the goal.

To reproduce, simply add the following debug spew and sleep lines to the current indigo-devel goal_manager_imp.h, re-compile the testing actionlib client, and run the server and client. The waitForResult() call in the test client will either hang indefinitely is a 0 timeout is used, or timeout without a result if >0 timeout is used.

diff --git a/include/actionlib/client/goal_manager_imp.h b/include/actionlib/client/goal_manager_imp.h
index 4df1229..709e4a8 100644
--- a/include/actionlib/client/goal_manager_imp.h
+++ b/include/actionlib/client/goal_manager_imp.h
@@ -70,6 +70,10 @@ ClientGoalHandle<ActionSpec> GoalManager<ActionSpec>::initGoal(const Goal& goal,
   typedef CommStateMachine<ActionSpec> CommStateMachineT;
   boost::shared_ptr<CommStateMachineT> comm_state_machine(new CommStateMachineT(action_goal, transition_cb, feedback_cb));
 
+  ROS_WARN("Actionlib Client has sent goal to server.  It will now sleep for 5 seconds in order to uncover bug.");
+  ros::Duration(5.0).sleep();
+  ROS_WARN("Done Sleeping.  If Actionlib server already finished and returned result, then the Result will now be missed, and the Client will fail on waitForResult()");
+
   boost::recursive_mutex::scoped_lock lock(list_mutex_);
   typename ManagedListT::Handle list_handle = list_.add(comm_state_machine, boost::bind(&GoalManagerT::listElemDeleter, this, _1), guard_);

[mikaelarguedas]

Fixed by #64