From 0055efde40c6ec927770a8c1f4fc38bb9fb94f36 Mon Sep 17 00:00:00 2001
From: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
Date: Wed, 2 Mar 2016 22:09:16 -0500
Subject: [PATCH] sip: re-write re-registration and mem-leak fix
This re-write simplify the code by:
- using lambda for C-scoped callback and to remove static method usage.
- remove unused arguments.
- remove the need of allocation, that could lead to memory-leak.
- make sure to be C++11 standard compliant (see uniform_int_distribution).
- use constexpr than const for true constants
- put constantes at top of file for visibility.
Also fixes a memory-leak by non-delete new-allocated std::pair
during re-registration.
Change-Id: Ib01cbecc025de7773342f071a17fe914264a1e6e
Tuleap: #458
---
src/sip/sipaccount.cpp | 51 +++++++++++++++++++-----------------------
src/sip/sipaccount.h | 9 ++++----
2 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/src/sip/sipaccount.cpp b/src/sip/sipaccount.cpp
index 0879e29550..2dd9485909 100644
--- a/src/sip/sipaccount.cpp
+++ b/src/sip/sipaccount.cpp
@@ -82,8 +82,10 @@ using yaml_utils::parseValue;
using yaml_utils::parseVectorMap;
using sip_utils::CONST_PJ_STR;
-static const int MIN_REGISTRATION_TIME = 60;
-static const int DEFAULT_REGISTRATION_TIME = 3600;
+static constexpr int MIN_REGISTRATION_TIME = 60; // seconds
+static constexpr unsigned DEFAULT_REGISTRATION_TIME = 3600; // seconds
+static constexpr unsigned REGISTRATION_FIRST_RETRY_INTERVAL = 60; // seconds
+static constexpr unsigned REGISTRATION_RETRY_INTERVAL = 300; // seconds
static const char *const VALID_TLS_PROTOS[] = {"Default", "TLSv1.2", "TLSv1.1", "TLSv1", "SSLv3"};
constexpr const char * const SIPAccount::ACCOUNT_TYPE;
@@ -1110,13 +1112,13 @@ SIPAccount::onRegister(pjsip_regc_cbparam *param)
case PJSIP_SC_BAD_GATEWAY:
case PJSIP_SC_SERVICE_UNAVAILABLE:
case PJSIP_SC_SERVER_TIMEOUT:
- scheduleReregistration(link_->getEndpoint());
+ scheduleReregistration();
break;
default:
/* Global failure */
if (PJSIP_IS_STATUS_IN_CLASS(param->code, 600))
- scheduleReregistration(link_->getEndpoint());
+ scheduleReregistration();
}
setRegistrationExpire(param->expiration);
}
@@ -2001,30 +2003,23 @@ SIPAccount::checkNATAddress(pjsip_regc_cbparam *param, pj_pool_t *pool)
/* Auto re-registration timeout callback */
void
-SIPAccount::autoReregTimerCb(pj_timer_heap_t * /*th*/, pj_timer_entry *te)
+SIPAccount::autoReregTimerCb()
{
- auto context = static_cast<std::pair<SIPAccount *, pjsip_endpoint *> *>(te->user_data);
- SIPAccount *acc = context->first;
- pjsip_endpoint *endpt = context->second;
-
- /* Check if the reregistration timer is still valid, e.g: while waiting
+ /* Check if the re-registration timer is still valid, e.g: while waiting
* timeout timer application might have deleted the account or disabled
* the auto-reregistration.
*/
- if (not acc->auto_rereg_.active) {
- delete context;
+ if (not auto_rereg_.active)
return;
- }
/* Start re-registration */
- acc->auto_rereg_.attempt_cnt++;
+ ++auto_rereg_.attempt_cnt;
try {
- acc->sendRegister();
- } catch (const VoipLinkException &e) {
- RING_ERR("%s", e.what());
- acc->scheduleReregistration(endpt);
+ sendRegister();
+ } catch (const VoipLinkException& e) {
+ RING_ERR("Exception during SIP registration: %s", e.what());
+ scheduleReregistration();
}
- delete context;
}
/* Schedule reregistration for specified account. Note that the first
@@ -2032,7 +2027,7 @@ SIPAccount::autoReregTimerCb(pj_timer_heap_t * /*th*/, pj_timer_entry *te)
* Also note that this function should be called within PJSUA mutex.
*/
void
-SIPAccount::scheduleReregistration(pjsip_endpoint *endpt)
+SIPAccount::scheduleReregistration()
{
if (!isUsable())
return;
@@ -2040,21 +2035,21 @@ SIPAccount::scheduleReregistration(pjsip_endpoint *endpt)
/* Cancel any re-registration timer */
if (auto_rereg_.timer.id) {
auto_rereg_.timer.id = PJ_FALSE;
- pjsip_endpt_cancel_timer(endpt, &auto_rereg_.timer);
+ pjsip_endpt_cancel_timer(link_->getEndpoint(), &auto_rereg_.timer);
}
/* Update re-registration flag */
auto_rereg_.active = PJ_TRUE;
/* Set up timer for reregistration */
- auto_rereg_.timer.cb = &SIPAccount::autoReregTimerCb;
- auto_rereg_.timer.user_data = new std::pair<SIPAccount *, pjsip_endpoint *>(this, endpt);
+ auto_rereg_.timer.cb = [](pj_timer_heap_t* /*th*/, pj_timer_entry* te) {
+ static_cast<SIPAccount*>(te->user_data)->autoReregTimerCb();
+ };
+ auto_rereg_.timer.user_data = this;
- /* Reregistration attempt. The first attempt will be done immediately. */
+ /* Reregistration attempt. The first attempt will be done sooner */
pj_time_val delay;
- const int FIRST_RETRY_INTERVAL = 60;
- const int RETRY_INTERVAL = 300;
- delay.sec = auto_rereg_.attempt_cnt ? RETRY_INTERVAL : FIRST_RETRY_INTERVAL;
+ delay.sec = auto_rereg_.attempt_cnt ? REGISTRATION_RETRY_INTERVAL : REGISTRATION_FIRST_RETRY_INTERVAL;
delay.msec = 0;
/* Randomize interval by +/- 10 secs */
@@ -2069,7 +2064,7 @@ SIPAccount::scheduleReregistration(pjsip_endpoint *endpt)
RING_WARN("Scheduling re-registration retry in %ld seconds..", delay.sec);
auto_rereg_.timer.id = PJ_TRUE;
- if (pjsip_endpt_schedule_timer(endpt, &auto_rereg_.timer, &delay) != PJ_SUCCESS)
+ if (pjsip_endpt_schedule_timer(link_->getEndpoint(), &auto_rereg_.timer, &delay) != PJ_SUCCESS)
auto_rereg_.timer.id = PJ_FALSE;
}
diff --git a/src/sip/sipaccount.h b/src/sip/sipaccount.h
index 9f0ebc8483..ff38140cb5 100644
--- a/src/sip/sipaccount.h
+++ b/src/sip/sipaccount.h
@@ -466,8 +466,6 @@ class SIPAccount : public SIPAccountBase {
*/
void supportPresence(int function, bool enable);
- void scheduleReregistration(pjsip_endpoint *endpt);
-
/**
* Implementation of Account::newOutgoingCall()
* Note: keep declaration before newOutgoingCall template.
@@ -546,10 +544,11 @@ class SIPAccount : public SIPAccountBase {
unsigned attempt_cnt; /**< Attempt counter. */
} auto_rereg_; /**< Reregister/reconnect data. */
- std::uniform_int_distribution<decltype(pj_time_val::msec)> delay10ZeroDist_ {-10000, 10000};
- std::uniform_int_distribution<decltype(pj_time_val::msec)> delay10PosDist_ {0, 10000};
+ std::uniform_int_distribution<int> delay10ZeroDist_ {-10000, 10000};
+ std::uniform_int_distribution<unsigned int> delay10PosDist_ {0, 10000};
- static void autoReregTimerCb(pj_timer_heap_t *th, pj_timer_entry *te);
+ void scheduleReregistration();
+ void autoReregTimerCb();
/**
* Map of credential for this account
--
GitLab