How can I prevent a timer from calling a function in a remote class?

Question

I have a problem in a piece of real code where a function belonging to a remote class is called boost::asio::deadline_timer , sometimes leading to a segmentation error.

The problem I am facing is that deadline_timer is removed from another timer on the same io_service. Removing the first deadline_timer will cause one final call to the function that will be launched, with the error boost::asio::error::operation_aborted . However, this can only be scheduled on the (same) io_service after the deletion is complete, but by then the object has already been deleted and therefore is no longer valid.

So my question is: how can I prevent this?

The following is a simplified example with the same error:

 //============================================================================ // Name : aTimeToKill.cpp // Author : Pelle // Description : Delete an object using a timer, from a timer //============================================================================ #include <iostream> #include <boost/function.hpp> #include <boost/bind.hpp> #include <boost/asio.hpp> #include <boost/thread.hpp> using namespace std; using namespace boost; struct TimeBomb { bool m_active; asio::deadline_timer m_runTimer; TimeBomb(boost::asio::io_service& ioService) : m_active(true) , m_runTimer(ioService) { cout << "Bomb placed @"<< hex << (int)this << endl; m_runTimer.expires_from_now(boost::posix_time::millisec(1000)); m_runTimer.async_wait(boost::bind(&TimeBomb::executeStepFunction, this, _1)); } ~TimeBomb() { m_active = false; m_runTimer.cancel(); cout << "Bomb defused @"<< hex << (int)this << endl; } void executeStepFunction(const boost::system::error_code& error) { // Canceled timer if (error == boost::asio::error::operation_aborted) { std::cout << "Timer aborted: " << error.message() << " @" << std::hex << (int)this << std::endl; return; } if (m_active) { // Schedule next step cout << "tick .." <<endl; m_runTimer.expires_from_now( boost::posix_time::millisec(1000)); m_runTimer.async_wait(boost::bind(&TimeBomb::executeStepFunction, this, _1)); } } }; struct BomberMan { asio::deadline_timer m_selfDestructTimer; TimeBomb* myBomb; BomberMan(boost::asio::io_service& ioService) : m_selfDestructTimer(ioService) { cout << "BomberMan ready " << endl; myBomb = new TimeBomb(ioService); m_selfDestructTimer.expires_from_now(boost::posix_time::millisec(10500)); m_selfDestructTimer.async_wait(boost::bind(&BomberMan::defuseBomb, this, _1)); } void defuseBomb(const boost::system::error_code& error) { cout << "Defusing TimeBomb" << endl; delete myBomb; } }; int main() { boost::asio::io_service m_ioService; BomberMan* b = new BomberMan(m_ioService); m_ioService.run(); return 0; } ./aTimeToKill BomberMan ready Bomb placed @9c27198 tick .. tick .. tick .. tick .. tick .. tick .. tick .. tick .. tick .. tick .. Defusing TimeBomb Bomb defused @9c27198 Timer aborted: Operation canceled @9c27198 

The last line is printed after deletion, illustrating my problem.

Answer 1

A typical recipe for solving this problem is to use shared_ptr

 #include <boost/asio.hpp> #include <boost/bind.hpp> #include <boost/enable_shared_from_this.hpp> #include <boost/shared_ptr.hpp> #include <iostream> using namespace std; struct TimeBomb : public boost::enable_shared_from_this<TimeBomb> { bool m_active; boost::asio::deadline_timer m_runTimer; TimeBomb(boost::asio::io_service& ioService) : m_active(true) , m_runTimer(ioService) { cout << "Bomb placed @"<< hex << this << endl; m_runTimer.expires_from_now(boost::posix_time::millisec(1000)); } void start() { m_runTimer.async_wait(boost::bind(&TimeBomb::executeStepFunction, shared_from_this(), _1)); } void stop() { m_runTimer.cancel(); } ~TimeBomb() { m_active = false; m_runTimer.cancel(); cout << "Bomb defused @"<< hex << this << endl; } void executeStepFunction(const boost::system::error_code& error) { // Canceled timer if (error == boost::asio::error::operation_aborted) { std::cout << "Timer aborted: " << error.message() << " @" << std::hex << this << std::endl; return; } if (m_active) { // Schedule next step cout << "tick .." <<endl; m_runTimer.expires_from_now( boost::posix_time::millisec(1000)); m_runTimer.async_wait(boost::bind(&TimeBomb::executeStepFunction, shared_from_this(), _1)); } } }; struct BomberMan { boost::asio::deadline_timer m_selfDestructTimer; boost::shared_ptr<TimeBomb> myBomb; BomberMan(boost::asio::io_service& ioService) : m_selfDestructTimer(ioService) { cout << "BomberMan ready " << endl; myBomb.reset( new TimeBomb(ioService) ); myBomb->start(); m_selfDestructTimer.expires_from_now(boost::posix_time::millisec(10500)); m_selfDestructTimer.async_wait(boost::bind(&BomberMan::defuseBomb, this, _1)); } void defuseBomb(const boost::system::error_code& error) { cout << "Defusing TimeBomb" << endl; myBomb->stop(); } }; int main() { boost::asio::io_service m_ioService; BomberMan* b = new BomberMan(m_ioService); m_ioService.run(); return 0; } 

Answer 2

This is why you have boost::shared_ptr and boost::enable_shared_from_this . Inherit the TimeBomb class from boost::enable_shared_from_this as follows:

 struct TimeBomb : public boost::enable_shared_from_this< TimeBomb > { ... } 

Create a shared ptr instead of open ptr:

 boost::shared_ptr< TimeBomb > myBomb; ... myBomb.reset( new TimeBomb(ioService) ); 

Finally, in TimeBomb use shared_from_this() instead of this to create handlers.

 m_runTimer.async_wait( boost::bind( &TimeBomb::executeStepFunction, shared_from_this(), _1)); 

And, of course, the TimeBomb class should set the cancel method, through which you cancel the async operation, and not by deleting or in this case reset shared_ptr.

Answer 3

Sam Miller’s shared_ptr response works because using shared_ptr holds a TimeBomb for the duration of a BomberMan. This may be good for you, or it may not be.

A suggestion for a more complete solution would be to get TimeBomb instances from the factory, which you then release before you finish, instead of adding and removing them explicitly (holding them as standard pointers, not shared_ptrs, since you don't own them even if you control the life cycle). factory can keep them hanging until they are undone, then delete them for you. Keep the Sam (Miller) Stop () function as is.

To implement this, take the factory out of the interface along the lines

 class ITimeBombObserver { public: virtual void AllOperationsComplete(TimeBomb& TmBmb)=0; }; 

Pass your factory to each TimeBomb as an ITimeBombObserver when building and cancel the TimeBomb function with this function. factory can clean up the “used” TimeBombs every time it is created or released, or using a scheduled cleanup or some other method, depending on what is most suitable for your application.

Using this method, your BomberMan does not even need to explicitly allocate a TimeBomb in defuseBomb (), if it does not want it, the stop () call can be automatically released (although in this case you should still indicate the pointer as it is actually unusable at the moment). Whether this is a good idea or not depends on your real problem, so I will leave it to you to decide.

Answer 4

For a really simple fix, how about this? (I just turned on the bit you need to change)

This works because you only get access to the stack variables when you cancel the timer. You really don't need to pay attention to the handler at all in the destructor, of course, but I assume that your real code requires this for any reason.

 ~TimeBomb() { m_active = false; executeStepFunction(boost::asio::error::interrupted); m_runTimer.cancel(); cout << "Bomb defused @"<< hex << (int)this << endl; } void executeStepFunction(const boost::system::error_code& error) { // Canceled timer if (error == boost::asio::error::operation_aborted) { return; } if (error == boost::asio::error::interrupted) { std::cout << "Timer aborted: " << error.message() << " @" << std::hex << (int)this << std::endl; return; } ...