From 272aee93d2bd0a0f55ff75ea58c464529eab80fa Mon Sep 17 00:00:00 2001 From: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com> Date: Wed, 4 Feb 2015 01:29:07 -0500 Subject: [PATCH] manager: fix event loop implementation Event loop duplicate event handler map before looping on it. So unregister an handler is not sync in this copy and can call dead stuff. Notice also registerEventHandler really replace the previous registered handler as it's expected. Refs #65401 Change-Id: Id2c27dd8ca42ee2a29fa3237bb836385deec11cd --- daemon/src/managerimpl.cpp | 26 ++++++++++++++++++++------ daemon/src/managerimpl.h | 3 ++- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/daemon/src/managerimpl.cpp b/daemon/src/managerimpl.cpp index a58684849e..9ca0827a31 100644 --- a/daemon/src/managerimpl.cpp +++ b/daemon/src/managerimpl.cpp @@ -1351,27 +1351,41 @@ ManagerImpl::removeStream(Call& call) getRingBufferPool().unBindAll(call_id); } +// Not thread-safe, SHOULD be called in same thread that run poolEvents() void ManagerImpl::registerEventHandler(uintptr_t handlerId, EventHandler handler) { - eventHandlerMap_.insert(std::make_pair(handlerId, handler)); + eventHandlerMap_[handlerId] = handler; } +// Not thread-safe, SHOULD be called in same thread that run poolEvents() void ManagerImpl::unregisterEventHandler(uintptr_t handlerId) { - eventHandlerMap_.erase(handlerId); + auto iter = eventHandlerMap_.find(handlerId); + if (iter != eventHandlerMap_.end()) { + if (iter == nextEventHandler_) + nextEventHandler_ = eventHandlerMap_.erase(iter); + else + eventHandlerMap_.erase(iter); + } } // Must be invoked periodically by a timer from the main event loop void ManagerImpl::pollEvents() { - // Make a copy of handlers map as handlers can modify this map - const auto handlers = eventHandlerMap_; - for (const auto& it : handlers) { + auto iter = eventHandlerMap_.begin(); + while (iter != eventHandlerMap_.end()) { if (finished_) return; - it.second(); + + // WARN: following callback can do anything and typically + // calls (un)registerEventHandler. + // Think twice before modify this code. + + nextEventHandler_ = std::next(iter); + iter->second(); + iter = nextEventHandler_; } } diff --git a/daemon/src/managerimpl.h b/daemon/src/managerimpl.h index bb7a4a6f88..0371145f71 100644 --- a/daemon/src/managerimpl.h +++ b/daemon/src/managerimpl.h @@ -968,7 +968,8 @@ class ManagerImpl { private: NON_COPYABLE(ManagerImpl); - std::map<uintptr_t, EventHandler> eventHandlerMap_{}; + std::map<uintptr_t, EventHandler> eventHandlerMap_; + decltype(eventHandlerMap_)::iterator nextEventHandler_; /** * Test if call is a valid call, i.e. have been created and stored in -- GitLab