ros / actionlib

Provides a standardized interface for interfacing with preemptable tasks. Examples of this include moving the base to a target location, performing a laser scan and returning the resulting point cloud, detecting the handle of a door, etc.
http://www.ros.org/wiki/actionlib
96 stars 154 forks source link

Segmentation Fault in Simple Action Client #76

Closed johaq closed 7 years ago

johaq commented 7 years ago

I am getting a segmentation fault when getState() in the simple action client is called. Extract from the gdb backtrace:

[Switching to Thread 0x7fff9effd700 (LWP 21180)]
0x00000000004f28b1 in boost::unique_lock<boost::recursive_mutex>::~unique_lock
    (this=<optimized out>, __in_chrg=<optimized out>)
    at /usr/include/boost/thread/lock_types.hpp:329
329           if (owns_lock())
(gdb) bt
Python Exception <class 'AttributeError'> module 'types' has no attribute 'InstanceType': 
#0  0x00000000004f28b1 in boost::unique_lock<boost::recursive_mutex>::~unique_lock (this=<optimized out>, __in_chrg=<optimized out>)
    at /usr/include/boost/thread/lock_types.hpp:329
Python Exception <class 'NameError'> Installation error: gdb.execute_unwinders function is missing: 
#1  actionlib::ClientGoalHandle<move_base_msgs::MoveBaseAction_<std::allocator<void> > >::getCommState (this=this@entry=0x7ffd58)
    at /opt/ros/kinetic/include/actionlib/client/client_goal_handle_imp.h:109
Python Exception <class 'NameError'> Installation error: gdb.execute_unwinders function is missing: 
#2  0x00000000004f38ba in actionlib::SimpleActionClient<move_base_msgs::MoveBaseAction_<std::allocator<void> > >::getState (this=0x7ffca0)
    at /opt/ros/kinetic/include/actionlib/client/simple_action_client.h:345
Python Exception <class 'NameError'> Installation error: gdb.execute_unwinders function is missing: 
#3  0x00000000004df28e in ros4rsb::NavigationServer::stop (this=0x7f7b80)
    at /media/local_ci/jenkins/jobs/ros-tobi_ros4rsb-kinetic-ci-deploy-robocup-nightly/workspace/src/servers/NavigationServer.cpp:248

The code in NavigationServer.cpp:

246 void NavigationServer::stop() {
247    ROS_INFO("call stop\n");
248    if (this->moveBaseClient->getState() == SimpleClientGoalState::ACTIVE) {
249       stopping = true;
250       moveBaseClient->cancelAllGoals();
251    } else {
252        ROS_WARN("called stop but there is no goal running? stopping velocity commander");
253        this->velocityCommander->stop();
254     usleep(500);
255
256    }
257    ROS_INFO("called stop\n");
258
259}
johaq commented 7 years ago

I think I fixed this issue. In client_goal_handle_imp.h we have:


template<class ActionSpec>
void ClientGoalHandle<ActionSpec>::reset()
{
  if (active_)
  {
    DestructionGuard::ScopedProtector protector(*guard_);
    if (!protector.isProtected())
    {
      ROS_ERROR_NAMED("actionlib", "This action client associated with the goal handle has already been destructed. Ignoring this reset() call");
      return;
    }

    boost::recursive_mutex::scoped_lock lock(gm_->list_mutex_);
    list_handle_.reset();
    active_ = false;
    gm_ = NULL;
  }
}
template<class ActionSpec>
CommState ClientGoalHandle<ActionSpec>::getCommState() const
{
  if (!active_)
  {
    ROS_ERROR_NAMED("actionlib", "Trying to getCommState on an inactive ClientGoalHandle. You are incorrectly using a ClientGoalHandle");
    return CommState(CommState::DONE);
  }

  DestructionGuard::ScopedProtector protector(*guard_);
  if (!protector.isProtected())
  {
    ROS_ERROR_NAMED("actionlib", "This action client associated with the goal handle has already been destructed. Ignoring this getCommState() call");
    return CommState(CommState::DONE);
  }

  assert(gm_);
  boost::recursive_mutex::scoped_lock lock(gm_->list_mutex_);
  return list_handle_.getElem()->getCommState();
}

As you can see active is set to false in the reset method after acquiring the scoped_lock. But it is checked in getCommState before acquiring it. This lead to an error when one thread called a reset while another asked for the commState. Pull Request: #77

mikaelarguedas commented 7 years ago

fixed in #77