diff options
author | Kevin Smith <git@kismith.co.uk> | 2016-01-21 18:15:50 (GMT) |
---|---|---|
committer | Kevin Smith <git@kismith.co.uk> | 2016-01-21 18:24:25 (GMT) |
commit | 8480bdda831e4d18c1979f40badb1bc985e78865 (patch) | |
tree | 10c68a327666fd7e352be3ce54865e50f77a161b | |
parent | f377207cb896679b4eab9f6773d9d071700852ad (diff) | |
download | swift-8480bdda831e4d18c1979f40badb1bc985e78865.zip swift-8480bdda831e4d18c1979f40badb1bc985e78865.tar.bz2 |
Ensure that BoostTimers can't fire after stop()
Code review makes me think that if the timer fires at the same time as
stop() is called, it would be possible for the clearing of events from
the loop to happen before the event is put into the loop, leading to
the sort of crashes we've seen with timers firing and access exceptions.
I can't reproduce those to know if this fixes them, and I'm not even sure
I'm not being dense thinking this patch fixes a real issue.
Test-Information:
Unit tests pass
Change-Id: I76337d5556a9c3902d5c2f4da754ae657810d436
-rw-r--r-- | Swiften/Network/BoostTimer.cpp | 14 | ||||
-rw-r--r-- | Swiften/Network/BoostTimer.h | 1 |
2 files changed, 12 insertions, 3 deletions
diff --git a/Swiften/Network/BoostTimer.cpp b/Swiften/Network/BoostTimer.cpp index 30f5180..c30fe32 100644 --- a/Swiften/Network/BoostTimer.cpp +++ b/Swiften/Network/BoostTimer.cpp @@ -1,54 +1,62 @@ /* * Copyright (c) 2010-2015 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ #include <Swiften/Network/BoostTimer.h> #include <boost/asio.hpp> #include <boost/bind.hpp> #include <boost/date_time/posix_time/posix_time.hpp> #include <Swiften/EventLoop/EventLoop.h> namespace Swift { BoostTimer::BoostTimer(int milliseconds, boost::shared_ptr<boost::asio::io_service> service, EventLoop* eventLoop) : - timeout(milliseconds), ioService(service), eventLoop(eventLoop) { + timeout(milliseconds), ioService(service), eventLoop(eventLoop), shuttingDown(false) { timer.reset(new boost::asio::deadline_timer(*service)); } BoostTimer::~BoostTimer() { { boost::mutex::scoped_lock lockTimer(timerMutex); timer.reset(); } } void BoostTimer::start() { { boost::mutex::scoped_lock lockTimer(timerMutex); + shuttingDown = false; timer->expires_from_now(boost::posix_time::milliseconds(timeout)); timer->async_wait(boost::bind(&BoostTimer::handleTimerTick, shared_from_this(), boost::asio::placeholders::error)); } } void BoostTimer::stop() { { boost::mutex::scoped_lock lockTimer(timerMutex); + shuttingDown = true; timer->cancel(); + eventLoop->removeEventsFromOwner(shared_from_this()); } - eventLoop->removeEventsFromOwner(shared_from_this()); } void BoostTimer::handleTimerTick(const boost::system::error_code& error) { if (error) { assert(error == boost::asio::error::operation_aborted); } else { - eventLoop->postEvent(boost::bind(boost::ref(onTick)), shared_from_this()); + { + boost::mutex::scoped_lock lockTimer(timerMutex); + if (shuttingDown) { + return; + } + eventLoop->postEvent(boost::bind(boost::ref(onTick)), shared_from_this()); + } } } } diff --git a/Swiften/Network/BoostTimer.h b/Swiften/Network/BoostTimer.h index a246b75..3227031 100644 --- a/Swiften/Network/BoostTimer.h +++ b/Swiften/Network/BoostTimer.h @@ -16,32 +16,33 @@ #include <Swiften/EventLoop/EventOwner.h> #include <Swiften/Network/Timer.h> namespace Swift { class EventLoop; class SWIFTEN_API BoostTimer : public Timer, public EventOwner, public boost::enable_shared_from_this<BoostTimer> { public: typedef boost::shared_ptr<BoostTimer> ref; virtual ~BoostTimer(); static ref create(int milliseconds, boost::shared_ptr<boost::asio::io_service> service, EventLoop* eventLoop) { return ref(new BoostTimer(milliseconds, service, eventLoop)); } virtual void start(); virtual void stop(); private: BoostTimer(int milliseconds, boost::shared_ptr<boost::asio::io_service> service, EventLoop* eventLoop); void handleTimerTick(const boost::system::error_code& error); private: int timeout; boost::shared_ptr<boost::asio::io_service> ioService; boost::scoped_ptr<boost::asio::deadline_timer> timer; boost::mutex timerMutex; EventLoop* eventLoop; + bool shuttingDown; }; } |