Module: sems Branch: master Commit: a0f6d4ff1412f789651acedfccce7e3bf8eda13f URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sems/?a=commit;h=a0f6d4ff1412f789651acedfccce7e3bf8eda13f
Author: Raphael Coeffic <[email protected]> Committer: Raphael Coeffic <[email protected]> Date: Wed May 30 16:25:32 2012 +0200 b/f: fixes a possible memory leak in the event dispatcher. Until this fix, it was possible to add an event queue with local tag, called and via-branch-tag, but omitting those parameters while deleting the event queue, thus leaving the lookup entries in the id_lookup map. This is no longer possible as the id_lookup map is now managed automatically. The additional id used as an index in id_lookup is saved together with the main queue entry in the queue map. --- core/AmEventDispatcher.cpp | 94 ++++++++++++++++++++---------------------- core/AmEventDispatcher.h | 25 ++++++++--- core/AmSessionContainer.cpp | 8 +--- 3 files changed, 65 insertions(+), 62 deletions(-) diff --git a/core/AmEventDispatcher.cpp b/core/AmEventDispatcher.cpp index b64155c..5d3de69 100644 --- a/core/AmEventDispatcher.cpp +++ b/core/AmEventDispatcher.cpp @@ -66,7 +66,7 @@ bool AmEventDispatcher::addEventQueue(const string& local_tag, return false; } - queues[queue_bucket][local_tag] = q; + queues[queue_bucket][local_tag] = QueueEntry(q); queues_mut[queue_bucket].unlock(); return true; @@ -95,11 +95,11 @@ bool AmEventDispatcher::addEventQueue(const string& local_tag, } // try to find via id_lookup - unsigned int id_bucket = hash(callid, remote_tag); string id = callid+remote_tag; if(AmConfig::AcceptForkedDialogs){ id += via_branch; } + unsigned int id_bucket = hash(id); id_lookup_mut[id_bucket].lock(); @@ -110,7 +110,7 @@ bool AmEventDispatcher::addEventQueue(const string& local_tag, return false; } - queues[queue_bucket][local_tag] = q; + queues[queue_bucket][local_tag] = QueueEntry(q,id); id_lookup[id_bucket][id] = local_tag; id_lookup_mut[id_bucket].unlock(); @@ -122,29 +122,6 @@ bool AmEventDispatcher::addEventQueue(const string& local_tag, AmEventQueueInterface* AmEventDispatcher::delEventQueue(const string& local_tag) { AmEventQueueInterface* q = NULL; - - unsigned int queue_bucket = hash(local_tag); - - queues_mut[queue_bucket].lock(); - - EvQueueMapIter qi = queues[queue_bucket].find(local_tag); - if(qi != queues[queue_bucket].end()) { - - q = qi->second; - queues[queue_bucket].erase(qi); - } - queues_mut[queue_bucket].unlock(); - - return q; -} - -AmEventQueueInterface* AmEventDispatcher::delEventQueue(const string& local_tag, - const string& callid, - const string& remote_tag, - const string& via_branch) -{ - AmEventQueueInterface* q = NULL; - unsigned int queue_bucket = hash(local_tag); queues_mut[queue_bucket].lock(); @@ -152,25 +129,22 @@ AmEventQueueInterface* AmEventDispatcher::delEventQueue(const string& local_tag, EvQueueMapIter qi = queues[queue_bucket].find(local_tag); if(qi != queues[queue_bucket].end()) { - q = qi->second; - queues[queue_bucket].erase(qi); - - if(!callid.empty() && !remote_tag.empty() && !via_branch.empty()) { - unsigned int id_bucket = hash(callid, remote_tag); - string id = callid+remote_tag; - if(AmConfig::AcceptForkedDialogs){ - id += via_branch; - } - - id_lookup_mut[id_bucket].lock(); - - DictIter di = id_lookup[id_bucket].find(id); - if(di != id_lookup[id_bucket].end()) { - id_lookup[id_bucket].erase(di); - } - - id_lookup_mut[id_bucket].unlock(); + QueueEntry qe(qi->second); + queues[queue_bucket].erase(qi); + q = qe.q; + + if(!qe.id.empty()) { + unsigned int id_bucket = hash(qe.id); + + id_lookup_mut[id_bucket].lock(); + + DictIter di = id_lookup[id_bucket].find(qe.id); + if(di != id_lookup[id_bucket].end()) { + id_lookup[id_bucket].erase(di); } + + id_lookup_mut[id_bucket].unlock(); + } } queues_mut[queue_bucket].unlock(); @@ -187,7 +161,7 @@ bool AmEventDispatcher::post(const string& local_tag, AmEvent* ev) EvQueueMapIter it = queues[queue_bucket].find(local_tag); if(it != queues[queue_bucket].end()){ - it->second->postEvent(ev); + it->second.q->postEvent(ev); posted = true; } @@ -202,11 +176,11 @@ bool AmEventDispatcher::post(const string& callid, const string& via_branch, AmEvent* ev) { - unsigned int id_bucket = hash(callid, remote_tag); string id = callid+remote_tag; if(AmConfig::AcceptForkedDialogs){ id += via_branch; } + unsigned int id_bucket = hash(id); id_lookup_mut[id_bucket].lock(); @@ -235,7 +209,7 @@ bool AmEventDispatcher::broadcast(AmEvent* ev) EvQueueMapIter this_evq = it; it++; queues_mut[i].unlock(); - this_evq->second->postEvent(ev->clone()); + this_evq->second.q->postEvent(ev->clone()); queues_mut[i].lock(); posted = true; } @@ -259,10 +233,31 @@ bool AmEventDispatcher::empty() { return res; } +void AmEventDispatcher::dump() +{ + DBG("*** dumping Event dispatcher buckets ***\n"); + for (size_t i=0;i<EVENT_DISPATCHER_BUCKETS;i++) { + queues_mut[i].lock(); + if(!queues[i].empty()) { + DBG("queues[%lu].size() = %lu",i,queues[i].size()); + } + queues_mut[i].unlock(); + + id_lookup_mut[i].lock(); + if(!id_lookup[i].empty()) { + DBG("id_lookup[%lu].size() = %lu",i,id_lookup[i].size()); + } + id_lookup_mut[i].unlock(); + } + DBG("*** End of Event dispatcher bucket dump ***\n"); +} + void AmEventDispatcher::dispose() { if(_instance != NULL) { // todo: add locking here + _instance->dump(); + delete _instance; _instance = NULL; } @@ -276,11 +271,12 @@ bool AmEventDispatcher::postSipRequest(const AmSipRequest& req) bool posted = false; string callid = req.callid; string remote_tag = req.from_tag; - unsigned int id_bucket = hash(callid, remote_tag); + string id = callid+remote_tag; if(AmConfig::AcceptForkedDialogs){ id += req.via_branch; } + unsigned int id_bucket = hash(id); id_lookup_mut[id_bucket].lock(); @@ -299,7 +295,7 @@ bool AmEventDispatcher::postSipRequest(const AmSipRequest& req) EvQueueMapIter it = queues[queue_bucket].find(local_tag); if(it != queues[queue_bucket].end()){ - it->second->postEvent(new AmSipRequestEvent(req)); + it->second.q->postEvent(new AmSipRequestEvent(req)); posted = true; } diff --git a/core/AmEventDispatcher.h b/core/AmEventDispatcher.h index 8fb53ce..c0757bb 100644 --- a/core/AmEventDispatcher.h +++ b/core/AmEventDispatcher.h @@ -38,8 +38,22 @@ class AmEventDispatcher { public: - typedef std::map<string, AmEventQueueInterface*> EvQueueMap; - typedef EvQueueMap::iterator EvQueueMapIter; + struct QueueEntry { + AmEventQueueInterface* q; + string id; + + QueueEntry() + : q(NULL), id() {} + + QueueEntry(AmEventQueueInterface* q) + : q(q), id() {} + + QueueEntry(AmEventQueueInterface* q, string id) + : q(q), id(id){} + }; + + typedef std::map<string, QueueEntry> EvQueueMap; + typedef EvQueueMap::iterator EvQueueMapIter; typedef std::map<string,string> Dictionnary; typedef Dictionnary::iterator DictIter; @@ -96,12 +110,9 @@ public: AmEventQueueInterface* delEventQueue(const string& local_tag); - AmEventQueueInterface* delEventQueue(const string& local_tag, - const string& callid, - const string& remote_tag, - const string& via_branch); - bool empty(); + + void dump(); }; #endif diff --git a/core/AmSessionContainer.cpp b/core/AmSessionContainer.cpp index 4c7e435..9a876c0 100644 --- a/core/AmSessionContainer.cpp +++ b/core/AmSessionContainer.cpp @@ -193,10 +193,7 @@ void AmSessionContainer::stopAndQueue(AmSession* s) void AmSessionContainer::destroySession(AmSession* s) { AmEventQueueInterface* q = AmEventDispatcher::instance()-> - delEventQueue(s->getLocalTag(), - s->getCallID(), - s->getRemoteTag(), - s->getFirstBranch()); + delEventQueue(s->getLocalTag()); if(q) { stopAndQueue(s); @@ -328,8 +325,7 @@ void AmSessionContainer::startSessionUAS(AmSipRequest& req) session->start(); } catch (...) { AmEventDispatcher::instance()-> - delEventQueue(req.callid,req.from_tag,local_tag, - req.via_branch); + delEventQueue(local_tag); throw; } _______________________________________________ Semsdev mailing list [email protected] http://lists.iptel.org/mailman/listinfo/semsdev
