Commit f75cbf2e authored by Simon Morlat's avatar Simon Morlat

Fixes and optimizations

- fix leakage of ServerGroupChatRoom, resulting in the conference server using 100% cpu for several seconds just to find list subscription handlers.
- replace unefficient use of std::list managing list subscriptions by std::unordered_map
- fix very costly == operator of ConferenceId and IdentityAddress, as they were instanciating a Address (wrapping belle_sip_header_address_t) in toString(). This change should greatly improve the chat rooms load time.
parent 7f12f8a6
......@@ -83,7 +83,9 @@ IdentityAddress &IdentityAddress::operator= (const IdentityAddress &other) {
}
bool IdentityAddress::operator== (const IdentityAddress &other) const {
return asString() == other.asString();
L_D();
/* Scheme is not used for comparison. sip:toto@sip.linphone.org and sips:toto@sip.linphone.org refer to the same person. */
return d->username == other.getUsername() && d->domain == other.getDomain() && d->gruu == other.getGruu();
}
bool IdentityAddress::operator!= (const IdentityAddress &other) const {
......@@ -91,12 +93,21 @@ bool IdentityAddress::operator!= (const IdentityAddress &other) const {
}
bool IdentityAddress::operator< (const IdentityAddress &other) const {
return asString() < other.asString();
L_D();
int diff = d->username.compare(other.getUsername());
if (diff == 0){
diff = d->domain.compare(other.getDomain());
if (diff == 0){
diff = d->gruu.compare(other.getGruu());
}
}
return diff < 0;
}
bool IdentityAddress::isValid () const {
Address tmpAddress(*this);
return tmpAddress.isValid();
L_D();
return !d->scheme.empty() && !d->domain.empty();
}
const string &IdentityAddress::getScheme () const {
......@@ -149,8 +160,17 @@ IdentityAddress IdentityAddress::getAddressWithoutGruu () const {
}
string IdentityAddress::asString () const {
Address tmpAddress(*this);
return tmpAddress.asStringUriOnly();
L_D();
ostringstream res;
res << d->scheme << ":";
if (!d->username.empty()){
res << d->username << "@";
}
res << d->domain;
if (!d->gruu.empty()){
res << ";gr=" << d->gruu;
}
return res.str();
}
LINPHONE_END_NAMESPACE
......@@ -207,7 +207,16 @@ void ServerGroupChatRoomPrivate::requestDeletion(){
for (auto participant : q->getParticipants()){
unSubscribeRegistrationForParticipant(participant->getAddress());
}
if (registrationSubscriptions.size() > 0){
lError() << q << " still " << registrationSubscriptions.size() << " registration subscriptions pending while deletion is requested.";
}
chatRoomListener->onChatRoomDeleteRequested(q->getSharedFromThis());
/*
* The LinphoneChatRoom (C object) is also built when the ServerGroupChatRoom is created, which is unnecessary here, but
* this is consistent with other kind of chatrooms. Consequence is that we have to free it in order to remove the last reference
* to the C++ object. */
LinphoneChatRoom * cChatRoom = L_GET_C_BACK_PTR(q);
if (cChatRoom) linphone_chat_room_unref(cChatRoom);
}
/*
......@@ -764,6 +773,7 @@ void ServerGroupChatRoomPrivate::finalizeCreation () {
IdentityAddress confAddr(qConference->getPrivate()->conferenceAddress);
conferenceId = ConferenceId(confAddr, confAddr);
qConference->getPrivate()->eventHandler->setConferenceId(conferenceId);
q->getCore()->getPrivate()->localListEventHandler->addHandler(qConference->getPrivate()->eventHandler.get());
lInfo() << q << " created";
// Let the SIP stack set the domain and the port
shared_ptr<Participant> me = q->getMe();
......@@ -1099,7 +1109,6 @@ ServerGroupChatRoom::ServerGroupChatRoom (const shared_ptr<Core> &core, SalCallO
: ChatRoom(*new ServerGroupChatRoomPrivate, core, ConferenceId(), ChatRoomParams::getDefaults(core)),
LocalConference(getCore(), IdentityAddress(linphone_proxy_config_get_conference_factory_uri(linphone_core_get_default_proxy_config(core->getCCore()))), nullptr) {
L_D();
L_D_T(LocalConference, dConference);
LocalConference::setSubject(op->getSubject());
const char *oneToOneChatRoomStr = sal_custom_header_find(op->getRecvCustomHeaders(), "One-To-One-Chat-Room");
......@@ -1113,7 +1122,6 @@ LocalConference(getCore(), IdentityAddress(linphone_proxy_config_get_conference_
shared_ptr<CallSession> session = getMe()->getPrivate()->createSession(*this, nullptr, false, d);
session->configure(LinphoneCallIncoming, nullptr, op, Address(op->getFrom()), Address(op->getTo()));
getCore()->getPrivate()->localListEventHandler->addHandler(dConference->eventHandler.get());
}
ServerGroupChatRoom::ServerGroupChatRoom (
......@@ -1139,12 +1147,14 @@ LocalConference(getCore(), peerAddress, nullptr) {
ServerGroupChatRoom::~ServerGroupChatRoom () {
L_D_T(LocalConference, dConference);
try {
if (getCore()->getPrivate()->localListEventHandler)
getCore()->getPrivate()->localListEventHandler->removeHandler(dConference->eventHandler.get());
} catch (const bad_weak_ptr &) {
// Unable to unregister listener here. Core is destroyed and the listener doesn't exist.
lInfo() << this << " destroyed.";
if (dConference->eventHandler->getConferenceId().isValid()){
try {
if (getCore()->getPrivate()->localListEventHandler)
getCore()->getPrivate()->localListEventHandler->removeHandler(dConference->eventHandler.get());
} catch (const bad_weak_ptr &) {
// Unable to unregister listener here. Core is destroyed and the listener doesn't exist.
}
}
};
......
......@@ -66,7 +66,6 @@ namespace std {
template<>
struct hash<LinphonePrivate::ConferenceId> {
std::size_t operator() (const LinphonePrivate::ConferenceId &conferenceId) const {
if (!conferenceId.isValid()) return std::size_t(-1);
return hash<string>()(conferenceId.getPeerAddress().asString()) ^
(hash<string>()(conferenceId.getLocalAddress().asString()) << 1);
}
......
......@@ -572,7 +572,7 @@ void LocalConferenceEventHandler::setConferenceId (const ConferenceId &conferenc
d->conferenceId = conferenceId;
}
ConferenceId LocalConferenceEventHandler::getConferenceId () const {
const ConferenceId &LocalConferenceEventHandler::getConferenceId () const {
L_D();
return d->conferenceId;
}
......
......@@ -54,7 +54,7 @@ public:
void setLastNotify (unsigned int lastNotify);
void setConferenceId (const ConferenceId &conferenceId);
ConferenceId getConferenceId () const;
const ConferenceId &getConferenceId () const;
std::string getNotifyForId (int notifyId, bool oneToOne = false);
......
......@@ -67,8 +67,8 @@ void LocalConferenceListEventHandler::notifyResponseCb (const LinphoneEvent *ev)
if (linphone_event_get_reason(ev) != LinphoneReasonNone)
return;
for (const auto &handler : listHandler->handlers) {
linphone_event_cbs_set_user_data(cbs, handler->getPrivate());
for (const auto &p : listHandler->handlers) {
linphone_event_cbs_set_user_data(cbs, p.second->getPrivate());
LocalConferenceEventHandlerPrivate::notifyResponseCb(ev);
}
linphone_event_cbs_set_user_data(cbs, nullptr);
......@@ -212,35 +212,37 @@ void LocalConferenceListEventHandler::subscribeReceived (LinphoneEvent *lev, con
void LocalConferenceListEventHandler::addHandler (LocalConferenceEventHandler *handler) {
if (!handler) {
lWarning() << "Trying to insert null handler in the local conference handler list";
lError() << "Trying to insert null handler in the local conference handler list";
return;
}
ConferenceId confId(handler->getConferenceId());
if(findHandler(confId)) {
lWarning() << "Trying to insert an already present handler in the local conference handler list: " << confId;
if(findHandler(handler->getConferenceId())) {
lError() << "Trying to insert an already present handler in the local conference handler list: " << handler->getConferenceId();
return;
}
handlers.push_back(handler);
handlers[handler->getConferenceId()] = handler;
}
void LocalConferenceListEventHandler::removeHandler (LocalConferenceEventHandler *handler) {
if (handler)
handlers.remove(handler);
if (handler){
auto it = handlers.find(handler->getConferenceId());
if (it != handlers.end()){
handlers.erase(it);
lInfo() << "Handler removed.";
}else{
lError() << "Handler not found in LocalConferenceListEventHandler.";
}
}else{
lError() << "Handler is null !";
}
}
LocalConferenceEventHandler *LocalConferenceListEventHandler::findHandler (const ConferenceId &conferenceId) const {
for (const auto &handler : handlers) {
if (handler->getConferenceId() == conferenceId)
return handler;
}
auto it = handlers.find(conferenceId);
if (it != handlers.end()) return (*it).second;
return nullptr;
}
const list<LocalConferenceEventHandler *> &LocalConferenceListEventHandler::getHandlers () const {
return handlers;
}
LINPHONE_END_NAMESPACE
......@@ -20,7 +20,7 @@
#ifndef _L_LOCAL_CONFERENCE_LIST_EVENT_HANDLER_H_
#define _L_LOCAL_CONFERENCE_LIST_EVENT_HANDLER_H_
#include <list>
#include <unordered_map>
#include "conference/conference-id.h"
#include "core/core-accessor.h"
......@@ -40,12 +40,11 @@ public:
void addHandler (LocalConferenceEventHandler *handler);
void removeHandler (LocalConferenceEventHandler *handler);
LocalConferenceEventHandler *findHandler (const ConferenceId &conferenceId) const;
const std::list<LocalConferenceEventHandler *> &getHandlers () const;
static void notifyResponseCb (const LinphoneEvent *ev);
private:
std::list<LocalConferenceEventHandler *> handlers;
std::unordered_map<ConferenceId, LocalConferenceEventHandler *> handlers;
};
......
......@@ -68,7 +68,7 @@ void RemoteConferenceListEventHandler::subscribe () {
lev = nullptr;
}
if (handlers.size() == 0)
if (handlers.empty())
return;
Content content;
......@@ -76,7 +76,8 @@ void RemoteConferenceListEventHandler::subscribe () {
Xsd::ResourceLists::ResourceLists rl = Xsd::ResourceLists::ResourceLists();
Xsd::ResourceLists::ListType l = Xsd::ResourceLists::ListType();
for (const auto &handler : handlers) {
for (const auto &p : handlers) {
RemoteConferenceEventHandler *handler = p.second;
const ConferenceId &conferenceId = handler->getConferenceId();
shared_ptr<AbstractChatRoom> cr = getCore()->findChatRoom(conferenceId);
if (!cr) {
......@@ -203,36 +204,46 @@ void RemoteConferenceListEventHandler::notifyReceived (const Content *notifyCont
// -----------------------------------------------------------------------------
RemoteConferenceEventHandler *RemoteConferenceListEventHandler::findHandler (const ConferenceId &conferenceId) const {
for (const auto &handler : handlers) {
if (handler->getConferenceId() == conferenceId)
return handler;
const auto it = handlers.find(conferenceId);
if (it != handlers.end()){
return (*it).second;
}
return nullptr;
}
const list<RemoteConferenceEventHandler *> &RemoteConferenceListEventHandler::getHandlers () const {
return handlers;
}
void RemoteConferenceListEventHandler::addHandler (RemoteConferenceEventHandler *handler) {
if (!handler) {
lWarning() << "Trying to insert null handler in the remote conference handler list";
return;
}
ConferenceId confId(handler->getConferenceId());
if(findHandler(confId)) {
lWarning() << "Trying to insert an already present handler in the remote conference handler list: " << confId;
if (!handler->getConferenceId().isValid()){
lError() << "RemoteConferenceListEventHandler::addHandler invalid handler.";
return;
}
handlers.push_back(handler);
if(findHandler(handler->getConferenceId())) {
lWarning() << "Trying to insert an already present handler in the remote conference handler list: " << handler->getConferenceId();
return;
}
handlers[handler->getConferenceId()] = handler;
}
void RemoteConferenceListEventHandler::removeHandler (RemoteConferenceEventHandler *handler) {
if (handler)
handlers.remove(handler);
if (!handler->getConferenceId().isValid()){
lError() << "RemoteConferenceListEventHandler::removeHandler() invalid handler.";
return;
}
if (handler){
auto it = handlers.find(handler->getConferenceId());
if (it != handlers.end()){
handlers.erase(it);
lInfo() << "Handler removed.";
}else{
lError() << "Handler not found in RemoteConferenceListEventHandler.";
}
}else{
lError() << "Handler is null !";
}
}
......
......@@ -20,8 +20,8 @@
#ifndef _L_REMOTE_CONFERENCE_LIST_EVENT_HANDLER_H_
#define _L_REMOTE_CONFERENCE_LIST_EVENT_HANDLER_H_
#include <list>
#include <map>
#include <unordered_map>
#include "linphone/types.h"
#include "linphone/utils/general.h"
......@@ -50,10 +50,9 @@ public:
void removeHandler (RemoteConferenceEventHandler *handler);
void clearHandlers ();
RemoteConferenceEventHandler *findHandler (const ConferenceId &conferenceId) const;
const std::list<RemoteConferenceEventHandler *> &getHandlers () const;
private:
std::list<RemoteConferenceEventHandler *> handlers;
std::unordered_map<ConferenceId, RemoteConferenceEventHandler *> handlers;
LinphoneEvent *lev = nullptr;
std::map<std::string, IdentityAddress> parseRlmi (const std::string &xmlBody) const;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment