From a3a5311f6878bc28d80fbd1025f1c8c7404a93f4 Mon Sep 17 00:00:00 2001
From: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
Date: Mon, 30 Jan 2017 10:51:22 -0500
Subject: [PATCH] sip: factorize dialog and invite session creation

SIP dialog and invite session creation code is duplicated
in SIPAccount and RingAccount call creation code.
This patch factorizes this code by moving identical parts
into SIPAccountBase base class.
The new common method is now CreateClientDialogAndInvite().

By the way notice that previous duplicated code was locked
the dialog. This extra lock it uneeded and wrong:
- wrong because pjsip_dlg_inc_lock() was used, but it should
be used only temporarily [1]
- uneeded as the dialog session counter is increased by the invite
session itself
The correct way is to call pjsip_dlg_inc_session if we need long
lock, that not our case here as we manage the invite session.

[1] http://www.pjsip.org/pjsip/docs/html/group__PJSIP__DIALOG.htm#ga5d87304615303ffbe20a09e0a3ff59df)

Change-Id: I8c2042b4c0278315cf76ea38affa8b90cab5cc7f
Tuleap: #64
---
 src/ringdht/ringaccount.cpp | 25 ++++--------------------
 src/sip/sipaccount.cpp      | 27 ++++----------------------
 src/sip/sipaccountbase.cpp  | 38 +++++++++++++++++++++++++++++++++++++
 src/sip/sipaccountbase.h    | 17 +++++++++++++++++
 src/sip/sipcall.cpp         |  1 -
 5 files changed, 63 insertions(+), 45 deletions(-)

diff --git a/src/ringdht/ringaccount.cpp b/src/ringdht/ringaccount.cpp
index f36de9ec2e..6ac9f60808 100644
--- a/src/ringdht/ringaccount.cpp
+++ b/src/ringdht/ringaccount.cpp
@@ -592,30 +592,13 @@ RingAccount::SIPStartCall(const std::shared_ptr<SIPCall>& call, IpAddr target)
              (int)pjContact.slen, pjContact.ptr, from.c_str(), toUri.c_str(),
              (int)pjTarget.slen, pjTarget.ptr);
 
-    pjsip_dialog *dialog = NULL;
-    if (pjsip_dlg_create_uac(pjsip_ua_instance(), &pjFrom, &pjContact, &pjTo, &pjTarget, &dialog) != PJ_SUCCESS) {
-        RING_ERR("Unable to create SIP dialogs for user agent client when "
-              "calling %s", toUri.c_str());
-        return false;
-    }
-
-    pj_str_t subj_hdr_name = CONST_PJ_STR("Subject");
-    pjsip_hdr* subj_hdr = (pjsip_hdr*) pjsip_parse_hdr(dialog->pool, &subj_hdr_name, (char *) "Phone call", 10, NULL);
-
-    pj_list_push_back(&dialog->inv_hdr, subj_hdr);
 
-    pjsip_inv_session* inv = nullptr;
-    if (pjsip_inv_create_uac(dialog, call->getSDP().getLocalSdpSession(), 0, &inv) != PJ_SUCCESS) {
-        RING_ERR("Unable to create invite session for user agent client");
+    auto local_sdp = call->getSDP().getLocalSdpSession();
+    pjsip_dialog* dialog {nullptr};
+    pjsip_inv_session* inv {nullptr};
+    if (!CreateClientDialogAndInvite(&pjFrom, &pjContact, &pjTo, &pjTarget, local_sdp, &dialog, &inv))
         return false;
-    }
-
-    if (!inv) {
-        RING_ERR("Call invite is not initialized");
-        return PJ_FALSE;
-    }
 
-    pjsip_dlg_inc_lock(inv->dlg);
     inv->mod_data[link_->getModId()] = call.get();
     call->inv.reset(inv);
 
diff --git a/src/sip/sipaccount.cpp b/src/sip/sipaccount.cpp
index 06a5477f56..da6a6fa4e3 100644
--- a/src/sip/sipaccount.cpp
+++ b/src/sip/sipaccount.cpp
@@ -339,31 +339,12 @@ SIPAccount::SIPStartCall(std::shared_ptr<SIPCall>& call)
     RING_DBG("contact header: %.*s / %s -> %s",
              (int)pjContact.slen, pjContact.ptr, from.c_str(), toUri.c_str());
 
-    pjsip_dialog *dialog = NULL;
-
-    if (pjsip_dlg_create_uac(pjsip_ua_instance(), &pjFrom, &pjContact, &pjTo, NULL, &dialog) != PJ_SUCCESS) {
-        RING_ERR("Unable to create SIP dialogs for user agent client when "
-              "calling %s", toUri.c_str());
+    auto local_sdp = call->getSDP().getLocalSdpSession();
+    pjsip_dialog* dialog {nullptr};
+    pjsip_inv_session* inv {nullptr};
+    if (!CreateClientDialogAndInvite(&pjFrom, &pjContact, &pjTo, nullptr, local_sdp, &dialog, &inv))
         return false;
-    }
-
-    auto subj_hdr_name = CONST_PJ_STR("Subject");
-    pjsip_hdr* subj_hdr = (pjsip_hdr*) pjsip_parse_hdr(dialog->pool, &subj_hdr_name, (char *) "Phone call", 10, NULL);
-
-    pj_list_push_back(&dialog->inv_hdr, subj_hdr);
-
-    pjsip_inv_session* inv = nullptr;
-    if (pjsip_inv_create_uac(dialog, call->getSDP().getLocalSdpSession(), 0, &inv) != PJ_SUCCESS) {
-        RING_ERR("Unable to create invite session for user agent client");
-        return false;
-    }
-
-    if (!inv) {
-        RING_ERR("Call invite is not initialized");
-        return PJ_FALSE;
-    }
 
-    pjsip_dlg_inc_lock(inv->dlg);
     inv->mod_data[link_->getModId()] = call.get();
     call->inv.reset(inv);
 
diff --git a/src/sip/sipaccountbase.cpp b/src/sip/sipaccountbase.cpp
index 5b516a2014..098995d53f 100644
--- a/src/sip/sipaccountbase.cpp
+++ b/src/sip/sipaccountbase.cpp
@@ -40,6 +40,7 @@
 #include "client/ring_signal.h"
 #include "string_utils.h"
 #include "fileutils.h"
+#include "sip_utils.h"
 
 #include <type_traits>
 
@@ -53,6 +54,43 @@ SIPAccountBase::SIPAccountBase(const std::string& accountID)
 
 SIPAccountBase::~SIPAccountBase() {}
 
+bool
+SIPAccountBase::CreateClientDialogAndInvite(const pj_str_t* from,
+                                            const pj_str_t* contact,
+                                            const pj_str_t* to,
+                                            const pj_str_t* target,
+                                            const pjmedia_sdp_session* local_sdp,
+                                            pjsip_dialog** dlg,
+                                            pjsip_inv_session** inv)
+{
+    if (pjsip_dlg_create_uac(pjsip_ua_instance(), from, contact, to, target, dlg) != PJ_SUCCESS) {
+        RING_ERR("Unable to create SIP dialogs for user agent client when calling %s", to->ptr);
+        return false;
+    }
+
+    auto dialog = *dlg;
+
+    {
+        // lock dialog until invite session creation; this one will own the dialog after
+        sip_utils::PJDialogLock dlg_lock {dialog};
+
+        // Append "Subject: Phone Call" header
+        auto subj_hdr_name = sip_utils::CONST_PJ_STR("Subject");
+        auto subj_hdr = reinterpret_cast<pjsip_hdr*>(pjsip_parse_hdr(dialog->pool,
+                                                                     &subj_hdr_name,
+                                                                     const_cast<char *>("Phone call"),
+                                                                     10, nullptr));
+        pj_list_push_back(&dialog->inv_hdr, subj_hdr);
+
+        if (pjsip_inv_create_uac(dialog, local_sdp, 0, inv) != PJ_SUCCESS) {
+            RING_ERR("Unable to create invite session for user agent client");
+            return false;
+        }
+    }
+
+    return true;
+}
+
 void
 SIPAccountBase::flush()
 {
diff --git a/src/sip/sipaccountbase.h b/src/sip/sipaccountbase.h
index c6e04c0316..01e538b863 100644
--- a/src/sip/sipaccountbase.h
+++ b/src/sip/sipaccountbase.h
@@ -41,6 +41,10 @@
 #include <map>
 #include <memory>
 
+struct pjsip_dialog;
+struct pjsip_inv_session;
+struct pjmedia_sdp_session;
+
 namespace ring {
 
 namespace Conf {
@@ -145,6 +149,19 @@ public:
         return false;
     }
 
+    /**
+     * Create UAC attached dialog and invite session
+     * @return true if success. false if failure and dlg and inv pointers
+     *         should not be considered as valid.
+     */
+    bool CreateClientDialogAndInvite(const pj_str_t* from,
+                                     const pj_str_t* contact,
+                                     const pj_str_t* to,
+                                     const pj_str_t* target,
+                                     const pjmedia_sdp_session* local_sdp,
+                                     pjsip_dialog** dlg,
+                                     pjsip_inv_session** inv);
+
     /**
      * Get the local interface name on which this account is bound.
      */
diff --git a/src/sip/sipcall.cpp b/src/sip/sipcall.cpp
index e1b7c94f26..b81b9fb3b5 100644
--- a/src/sip/sipcall.cpp
+++ b/src/sip/sipcall.cpp
@@ -1101,7 +1101,6 @@ SIPCall::InvSessionDeleter::operator ()(pjsip_inv_session* inv) const noexcept
     // prevent this from getting accessed in callbacks
     // RING_WARN: this is not thread-safe!
     inv->mod_data[getSIPVoIPLink()->getModId()] = nullptr;
-    pjsip_dlg_dec_lock(inv->dlg);
 }
 
 bool
-- 
GitLab