<p>Friendly Automation would like Kevin Harwell to <strong>review</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15163">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">AST-2020-001 - res_pjsip: Return dialog locked and referenced<br><br>pjproject returns the dialog locked and with a reference. However,<br>in Asterisk the method that handles this decrements the reference<br>and removes the lock prior to returning. This makes it possible,<br>under some circumstances, for another thread to free said dialog<br>before the thread that created it attempts to use it again. Of<br>course when the thread that created it tries to use a freed dialog<br>a crash can occur.<br><br>This patch makes it so Asterisk now returns the newly created<br>dialog both locked, and with an added reference. This allows the<br>caller to de-reference, and unlock the dialog when it is safe to<br>do so.<br><br>In the case of a new SIP Invite the lock, and reference are now<br>held for the entirety of the new invite handling process.<br>Otherwise it's possible for the dialog, or its dependent objects,<br>like the transaction, to disappear. For example if there is a TCP<br>transport error.<br><br>ASTERISK-29057 #close<br><br>Change-Id: I5ef645a47829596f402cf383dc02c629c618969e<br>---<br>M include/asterisk/res_pjsip.h<br>M res/res_pjsip.c<br>M res/res_pjsip_pubsub.c<br>M res/res_pjsip_session.c<br>4 files changed, 226 insertions(+), 26 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/63/15163/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h</span><br><span>index dee8d2d..e14eebc 100644</span><br><span>--- a/include/asterisk/res_pjsip.h</span><br><span>+++ b/include/asterisk/res_pjsip.h</span><br><span>@@ -1924,6 +1924,11 @@</span><br><span> /*!</span><br><span> * \brief General purpose method for creating a UAS dialog with an endpoint</span><br><span> *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \deprecated This function is unsafe (due to the returned object not being locked nor</span><br><span style="color: hsl(120, 100%, 40%);">+ * having its reference incremented) and should no longer be used. Instead</span><br><span style="color: hsl(120, 100%, 40%);">+ * use ast_sip_create_dialog_uas_locked so a properly locked and referenced</span><br><span style="color: hsl(120, 100%, 40%);">+ * object is returned.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span> * \param endpoint A pointer to the endpoint</span><br><span> * \param rdata The request that is starting the dialog</span><br><span> * \param[out] status On failure, the reason for failure in creating the dialog</span><br><span>@@ -1931,6 +1936,44 @@</span><br><span> pjsip_dialog *ast_sip_create_dialog_uas(const struct ast_sip_endpoint *endpoint, pjsip_rx_data *rdata, pj_status_t *status);</span><br><span> </span><br><span> /*!</span><br><span style="color: hsl(120, 100%, 40%);">+ * \brief General purpose method for creating a UAS dialog with an endpoint</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * This function creates and returns a locked, and referenced counted pjsip</span><br><span style="color: hsl(120, 100%, 40%);">+ * dialog object. The caller is thus responsible for freeing the allocated</span><br><span style="color: hsl(120, 100%, 40%);">+ * memory, decrementing the reference, and releasing the lock when done with</span><br><span style="color: hsl(120, 100%, 40%);">+ * the returned object.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \note The safest way to unlock the object, and decrement its reference is by</span><br><span style="color: hsl(120, 100%, 40%);">+ * calling pjsip_dlg_dec_lock. Alternatively, pjsip_dlg_dec_session can be</span><br><span style="color: hsl(120, 100%, 40%);">+ * used to decrement the reference only.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * The dialog is returned locked and with a reference in order to ensure that the</span><br><span style="color: hsl(120, 100%, 40%);">+ * dialog object, and any of its associated objects (e.g. transaction) are not</span><br><span style="color: hsl(120, 100%, 40%);">+ * untimely destroyed. For instance, that could happen when a transport error</span><br><span style="color: hsl(120, 100%, 40%);">+ * occurs.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * As long as the caller maintains a reference to the dialog there should be no</span><br><span style="color: hsl(120, 100%, 40%);">+ * worry that it might unknowningly be destroyed. However, once the caller unlocks</span><br><span style="color: hsl(120, 100%, 40%);">+ * the dialog there is a danger that some of the dialog's internal objects could</span><br><span style="color: hsl(120, 100%, 40%);">+ * be lost and/or compromised. For example, when the aforementioned transport error</span><br><span style="color: hsl(120, 100%, 40%);">+ * occurs the dialog's associated transaction gets destroyed (see pjsip_dlg_on_tsx_state</span><br><span style="color: hsl(120, 100%, 40%);">+ * in sip_dialog.c, and mod_inv_on_tsx_state in sip_inv.c).</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * In this case and before using the dialog again the caller should re-lock the</span><br><span style="color: hsl(120, 100%, 40%);">+ * dialog, check to make sure the dialog is still established, and the transaction</span><br><span style="color: hsl(120, 100%, 40%);">+ * still exists and has not been destroyed.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param endpoint A pointer to the endpoint</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param rdata The request that is starting the dialog</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[out] status On failure, the reason for failure in creating the dialog</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \retval A locked, and reference counted pjsip_dialog object.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \retval NULL on failure</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+pjsip_dialog *ast_sip_create_dialog_uas_locked(const struct ast_sip_endpoint *endpoint,</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_rx_data *rdata, pj_status_t *status);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/*!</span><br><span> * \brief General purpose method for creating an rdata structure using specific information</span><br><span> * \since 13.15.0</span><br><span> *</span><br><span>diff --git a/res/res_pjsip.c b/res/res_pjsip.c</span><br><span>index 90438d4..0067c15 100644</span><br><span>--- a/res/res_pjsip.c</span><br><span>+++ b/res/res_pjsip.c</span><br><span>@@ -3704,7 +3704,11 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-pjsip_dialog *ast_sip_create_dialog_uas(const struct ast_sip_endpoint *endpoint, pjsip_rx_data *rdata, pj_status_t *status)</span><br><span style="color: hsl(120, 100%, 40%);">+typedef pj_status_t (*create_dlg_uac)(pjsip_user_agent *ua, pjsip_rx_data *rdata,</span><br><span style="color: hsl(120, 100%, 40%);">+ const pj_str_t *contact, pjsip_dialog **p_dlg);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+static pjsip_dialog *create_dialog_uas(const struct ast_sip_endpoint *endpoint,</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_rx_data *rdata, pj_status_t *status, create_dlg_uac create_fun)</span><br><span> {</span><br><span> pjsip_dialog *dlg;</span><br><span> pj_str_t contact;</span><br><span>@@ -3739,11 +3743,7 @@</span><br><span> (type != PJSIP_TRANSPORT_UDP && type != PJSIP_TRANSPORT_UDP6) ? ";transport=" : "",</span><br><span> (type != PJSIP_TRANSPORT_UDP && type != PJSIP_TRANSPORT_UDP6) ? pjsip_transport_get_type_name(type) : "");</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-#ifdef HAVE_PJSIP_DLG_CREATE_UAS_AND_INC_LOCK</span><br><span style="color: hsl(0, 100%, 40%);">- *status = pjsip_dlg_create_uas_and_inc_lock(pjsip_ua_instance(), rdata, &contact, &dlg);</span><br><span style="color: hsl(0, 100%, 40%);">-#else</span><br><span style="color: hsl(0, 100%, 40%);">- *status = pjsip_dlg_create_uas(pjsip_ua_instance(), rdata, &contact, &dlg);</span><br><span style="color: hsl(0, 100%, 40%);">-#endif</span><br><span style="color: hsl(120, 100%, 40%);">+ *status = create_fun(pjsip_ua_instance(), rdata, &contact, &dlg);</span><br><span> if (*status != PJ_SUCCESS) {</span><br><span> char err[PJ_ERR_MSG_SIZE];</span><br><span> </span><br><span>@@ -3756,13 +3756,48 @@</span><br><span> dlg->sess_count++;</span><br><span> pjsip_dlg_set_transport(dlg, &selector);</span><br><span> dlg->sess_count--;</span><br><span style="color: hsl(0, 100%, 40%);">-#ifdef HAVE_PJSIP_DLG_CREATE_UAS_AND_INC_LOCK</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_dlg_dec_lock(dlg);</span><br><span style="color: hsl(0, 100%, 40%);">-#endif</span><br><span> </span><br><span> return dlg;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+pjsip_dialog *ast_sip_create_dialog_uas(const struct ast_sip_endpoint *endpoint, pjsip_rx_data *rdata, pj_status_t *status)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+#ifdef HAVE_PJSIP_DLG_CREATE_UAS_AND_INC_LOCK</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_dialog *dlg;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ dlg = create_dialog_uas(endpoint, rdata, status, pjsip_dlg_create_uas_and_inc_lock);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (dlg) {</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_dlg_dec_lock(dlg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return dlg;</span><br><span style="color: hsl(120, 100%, 40%);">+#else</span><br><span style="color: hsl(120, 100%, 40%);">+ return create_dialog_uas(endpoint, rdata, status, pjsip_dlg_create_uas);</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+pjsip_dialog *ast_sip_create_dialog_uas_locked(const struct ast_sip_endpoint *endpoint,</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_rx_data *rdata, pj_status_t *status)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+#ifdef HAVE_PJSIP_DLG_CREATE_UAS_AND_INC_LOCK</span><br><span style="color: hsl(120, 100%, 40%);">+ return create_dialog_uas(endpoint, rdata, status, pjsip_dlg_create_uas_and_inc_lock);</span><br><span style="color: hsl(120, 100%, 40%);">+#else</span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * This is put here in order to be compatible with older versions of pjproject.</span><br><span style="color: hsl(120, 100%, 40%);">+ * Best we can do in this case is immediately lock after getting the dialog.</span><br><span style="color: hsl(120, 100%, 40%);">+ * However, that does leave a "gap" between creating and locking.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_dialog *dlg;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ dlg = create_dialog_uas(endpoint, rdata, status, pjsip_dlg_create_uas);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (dlg) {</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_dlg_inc_lock(dlg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return dlg;</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> int ast_sip_create_rdata_with_contact(pjsip_rx_data *rdata, char *packet, const char *src_name, int src_port,</span><br><span> char *transport_type, const char *local_name, int local_port, const char *contact)</span><br><span> {</span><br><span>diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c</span><br><span>index 1142bbd..cf8baea 100644</span><br><span>--- a/res/res_pjsip_pubsub.c</span><br><span>+++ b/res/res_pjsip_pubsub.c</span><br><span>@@ -1477,7 +1477,7 @@</span><br><span> }</span><br><span> sub_tree->role = AST_SIP_NOTIFIER;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- dlg = ast_sip_create_dialog_uas(endpoint, rdata, dlg_status);</span><br><span style="color: hsl(120, 100%, 40%);">+ dlg = ast_sip_create_dialog_uas_locked(endpoint, rdata, dlg_status);</span><br><span> if (!dlg) {</span><br><span> if (*dlg_status != PJ_EEXISTS) {</span><br><span> ast_log(LOG_WARNING, "Unable to create dialog for SIP subscription\n");</span><br><span>@@ -1498,8 +1498,16 @@</span><br><span> }</span><br><span> </span><br><span> pjsip_evsub_create_uas(dlg, &pubsub_cb, rdata, 0, &sub_tree->evsub);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> subscription_setup_dialog(sub_tree, dlg);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * The evsub and subscription setup both add dialog refs, so the dialog ref that</span><br><span style="color: hsl(120, 100%, 40%);">+ * was added when the dialog was created (see ast_sip_create_dialog_uas_lock) can</span><br><span style="color: hsl(120, 100%, 40%);">+ * now be removed. The lock should no longer be needed so can be removed too.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_dlg_dec_lock(dlg);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #ifdef HAVE_PJSIP_EVSUB_GRP_LOCK</span><br><span> pjsip_evsub_add_ref(sub_tree->evsub);</span><br><span> #endif</span><br><span>diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c</span><br><span>index e8c8fdf..209e68c 100644</span><br><span>--- a/res/res_pjsip_session.c</span><br><span>+++ b/res/res_pjsip_session.c</span><br><span>@@ -3746,6 +3746,75 @@</span><br><span> return SIP_GET_DEST_EXTEN_NOT_FOUND;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*</span><br><span style="color: hsl(120, 100%, 40%);">+ * /internal</span><br><span style="color: hsl(120, 100%, 40%);">+ * /brief Process initial answer for an incoming invite</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * This function should only be called during the setup, and handling of a</span><br><span style="color: hsl(120, 100%, 40%);">+ * new incoming invite. Most, if not all of the time, this will be called</span><br><span style="color: hsl(120, 100%, 40%);">+ * when an error occurs and we need to respond as such.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * When a SIP session termination code is given for the answer it's assumed</span><br><span style="color: hsl(120, 100%, 40%);">+ * this call then will be the final bit of processing before ending session</span><br><span style="color: hsl(120, 100%, 40%);">+ * setup. As such, we've been holding a lock, and a reference on the invite</span><br><span style="color: hsl(120, 100%, 40%);">+ * session's dialog. So before returning this function removes that reference,</span><br><span style="color: hsl(120, 100%, 40%);">+ * and unlocks the dialog.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param inv_session The session on which to answer</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param rdata The original request</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param answer_code The answer's numeric code</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param terminate_code The termination code if the answer fails</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param notify Whether or not to call on_state_changed</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \retval 0 if invite successfully answered, -1 if an error occurred</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+static int new_invite_initial_answer(pjsip_inv_session *inv_session, pjsip_rx_data *rdata,</span><br><span style="color: hsl(120, 100%, 40%);">+ int answer_code, int terminate_code, pj_bool_t notify)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_tx_data *tdata = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ int res = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (inv_session->state != PJSIP_INV_STATE_DISCONNECTED) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (pjsip_inv_initial_answer(</span><br><span style="color: hsl(120, 100%, 40%);">+ inv_session, rdata, answer_code, NULL, NULL, &tdata) != PJ_SUCCESS) {</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_inv_terminate(inv_session, terminate_code ? terminate_code : answer_code, notify);</span><br><span style="color: hsl(120, 100%, 40%);">+ res = -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_inv_send_msg(inv_session, tdata);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (answer_code >= 300) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * A session is ending. The dialog has a reference that needs to be</span><br><span style="color: hsl(120, 100%, 40%);">+ * removed and holds a lock that needs to be unlocked before returning.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_dlg_dec_lock(inv_session->dlg);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return res;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/*</span><br><span style="color: hsl(120, 100%, 40%);">+ * /internal</span><br><span style="color: hsl(120, 100%, 40%);">+ * /brief Create and initialize a pjsip invite session</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ * pjsip_inv_session adds, and maintains a reference to the dialog upon a successful</span><br><span style="color: hsl(120, 100%, 40%);">+ * invite session creation until the session is destroyed. However, we'll wait to</span><br><span style="color: hsl(120, 100%, 40%);">+ * remove the reference that was added for the dialog when it gets created since we're</span><br><span style="color: hsl(120, 100%, 40%);">+ * not ready to unlock the dialog in this function.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * So, if this function successfully returns that means it returns with its newly</span><br><span style="color: hsl(120, 100%, 40%);">+ * created, and associated dialog locked and with two references (i.e. dialog's</span><br><span style="color: hsl(120, 100%, 40%);">+ * reference count should be 2).</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param endpoint A pointer to the endpoint</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param rdata The request that is starting the dialog</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \retval A pjsip invite session object</span><br><span style="color: hsl(120, 100%, 40%);">+ * \retval NULL on error</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span> static pjsip_inv_session *pre_session_setup(pjsip_rx_data *rdata, const struct ast_sip_endpoint *endpoint)</span><br><span> {</span><br><span> pjsip_tx_data *tdata;</span><br><span>@@ -3764,15 +3833,28 @@</span><br><span> }</span><br><span> return NULL;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- dlg = ast_sip_create_dialog_uas(endpoint, rdata, &dlg_status);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ dlg = ast_sip_create_dialog_uas_locked(endpoint, rdata, &dlg_status);</span><br><span> if (!dlg) {</span><br><span> if (dlg_status != PJ_EEXISTS) {</span><br><span> pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);</span><br><span> }</span><br><span> return NULL;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * The returned dialog holds a lock and has a reference added. Any paths where the</span><br><span style="color: hsl(120, 100%, 40%);">+ * dialog invite session is not returned must unlock the dialog and remove its reference.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (pjsip_inv_create_uas(dlg, rdata, NULL, options, &inv_session) != PJ_SUCCESS) {</span><br><span> pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * The acquired dialog holds a lock, and a reference. Since the dialog is not</span><br><span style="color: hsl(120, 100%, 40%);">+ * going to be returned here it must first be unlocked and de-referenced. This</span><br><span style="color: hsl(120, 100%, 40%);">+ * must be done prior to calling dialog termination.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_dlg_dec_lock(dlg);</span><br><span> pjsip_dlg_terminate(dlg);</span><br><span> return NULL;</span><br><span> }</span><br><span>@@ -3781,12 +3863,13 @@</span><br><span> inv_session->sdp_neg_flags = PJMEDIA_SDP_NEG_ALLOW_MEDIA_CHANGE;</span><br><span> #endif</span><br><span> if (pjsip_dlg_add_usage(dlg, &session_module, NULL) != PJ_SUCCESS) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (pjsip_inv_initial_answer(inv_session, rdata, 500, NULL, NULL, &tdata) != PJ_SUCCESS) {</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_inv_terminate(inv_session, 500, PJ_FALSE);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_inv_send_msg(inv_session, tdata);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Dialog's lock and a reference are removed in new_invite_initial_answer */</span><br><span style="color: hsl(120, 100%, 40%);">+ new_invite_initial_answer(inv_session, rdata, 500, 500, PJ_FALSE);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Remove 2nd reference added at inv_session creation */</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_dlg_dec_session(inv_session->dlg, &session_module);</span><br><span> return NULL;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> return inv_session;</span><br><span> }</span><br><span> </span><br><span>@@ -4003,7 +4086,6 @@</span><br><span> {</span><br><span> RAII_VAR(struct ast_sip_endpoint *, endpoint,</span><br><span> ast_pjsip_rdata_get_endpoint(rdata), ao2_cleanup);</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_tx_data *tdata = NULL;</span><br><span> pjsip_inv_session *inv_session = NULL;</span><br><span> struct ast_sip_session *session;</span><br><span> struct new_invite invite;</span><br><span>@@ -4016,27 +4098,48 @@</span><br><span> return;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * Upon a successful pre_session_setup the associated dialog is returned locked</span><br><span style="color: hsl(120, 100%, 40%);">+ * and with an added reference. Well actually two references. One added when the</span><br><span style="color: hsl(120, 100%, 40%);">+ * dialog itself was created, and another added when the pjsip invite session was</span><br><span style="color: hsl(120, 100%, 40%);">+ * created and the dialog was added to it.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * In order to ensure the dialog's, and any of its internal attributes, lifetimes</span><br><span style="color: hsl(120, 100%, 40%);">+ * we'll hold the lock and maintain the reference throughout the entire new invite</span><br><span style="color: hsl(120, 100%, 40%);">+ * handling process. See ast_sip_create_dialog_uas_locked for more details but,</span><br><span style="color: hsl(120, 100%, 40%);">+ * basically we do this to make sure a transport failure does not destroy the dialog</span><br><span style="color: hsl(120, 100%, 40%);">+ * and/or transaction out from underneath us between pjsip calls. Alternatively, we</span><br><span style="color: hsl(120, 100%, 40%);">+ * could probably release the lock if we needed to, but then we'd have to re-lock and</span><br><span style="color: hsl(120, 100%, 40%);">+ * check the dialog and transaction prior to every pjsip call.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * That means any off nominal/failure paths in this function must remove the associated</span><br><span style="color: hsl(120, 100%, 40%);">+ * dialog reference added at dialog creation, and remove the lock. As well the</span><br><span style="color: hsl(120, 100%, 40%);">+ * referenced pjsip invite session must be "cleaned up", which should also then</span><br><span style="color: hsl(120, 100%, 40%);">+ * remove its reference to the dialog at that time.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * Nominally we'll unlock the dialog, and release the reference when all new invite</span><br><span style="color: hsl(120, 100%, 40%);">+ * process handling has successfully completed.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #ifdef HAVE_PJSIP_INV_SESSION_REF</span><br><span> if (pjsip_inv_add_ref(inv_session) != PJ_SUCCESS) {</span><br><span> ast_log(LOG_ERROR, "Can't increase the session reference counter\n");</span><br><span style="color: hsl(0, 100%, 40%);">- if (inv_session->state != PJSIP_INV_STATE_DISCONNECTED) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (pjsip_inv_initial_answer(inv_session, rdata, 500, NULL, NULL, &tdata) == PJ_SUCCESS) {</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_inv_terminate(inv_session, 500, PJ_FALSE);</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_inv_send_msg(inv_session, tdata);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Dialog's lock and a reference are removed in new_invite_initial_answer */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!new_invite_initial_answer(inv_session, rdata, 500, 500, PJ_FALSE)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Terminate the session if it wasn't done in the answer */</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_inv_terminate(inv_session, 500, PJ_FALSE);</span><br><span> }</span><br><span> return;</span><br><span> }</span><br><span> #endif</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> session = ast_sip_session_alloc(endpoint, NULL, inv_session, rdata);</span><br><span> if (!session) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (pjsip_inv_initial_answer(inv_session, rdata, 500, NULL, NULL, &tdata) == PJ_SUCCESS) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Dialog's lock and reference are removed in new_invite_initial_answer */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!new_invite_initial_answer(inv_session, rdata, 500, 500, PJ_FALSE)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Terminate the session if it wasn't done in the answer */</span><br><span> pjsip_inv_terminate(inv_session, 500, PJ_FALSE);</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">- pjsip_inv_send_msg(inv_session, tdata);</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #ifdef HAVE_PJSIP_INV_SESSION_REF</span><br><span> pjsip_inv_dec_ref(inv_session);</span><br><span> #endif</span><br><span>@@ -4054,6 +4157,17 @@</span><br><span> invite.rdata = rdata;</span><br><span> new_invite(&invite);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * The dialog lock and reference added at dialog creation time must be</span><br><span style="color: hsl(120, 100%, 40%);">+ * maintained throughout the new invite process. Since we're pretty much</span><br><span style="color: hsl(120, 100%, 40%);">+ * done at this point with things it's safe to go ahead and remove the lock</span><br><span style="color: hsl(120, 100%, 40%);">+ * and the reference here. See ast_sip_create_dialog_uas_locked for more info.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * Note, any future functionality added that does work using the dialog must</span><br><span style="color: hsl(120, 100%, 40%);">+ * be done before this.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ pjsip_dlg_dec_lock(inv_session->dlg);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> ao2_ref(session, -1);</span><br><span> }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15163">change 15163</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/15163"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I5ef645a47829596f402cf383dc02c629c618969e </div>
<div style="display:none"> Gerrit-Change-Number: 15163 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>