SimpleActionServer destructor uses uninitialised variable

[pallgeuer]

While running valgrind on my node that involves a SimpleActionServer I got the following:

==10202== Conditional jump or move depends on uninitialised value(s)
==10202==    at 0x444B8F: actionlib::SimpleActionServer<robotcontrol::FadeTorqueAction_<std::allocator<void> > >::~SimpleActionServer() (simple_action_server_imp.h:141)
==10202==    by 0x4372A8: robotcontrol::RobotControl::~RobotControl() (robotcontrol.cpp:122)
==10202==    by 0x43025C: main (robotcontrol.cpp:679)

Even though I wasn't experiencing any problems per se I had a look into this. Indeed, execute_thread_ is not necessarily initialised in the class constructors (if no execute callback is passed) yet the destructor goes ahead and uses it:

template <class ActionSpec>
SimpleActionServer<ActionSpec>::~SimpleActionServer()
{
  if(execute_thread_)
    shutdown();
}

Changing that to if(execute_callback_ && execute_thread_) may suffice to resolve the issue.

[esteve]

@pallgeuer Thanks for reporting this bug. I created a pull request that initialized execute_thread_ to NULL. Could you test that #37 works and that valgrind no longer reports execute_thread_ as not being initialized?

Thanks.

[pallgeuer]

@esteve Yes, that resolves the issue, thanks.

[esteve]

@pallgeuer thanks for testing the PR, I just merged it.