<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7151">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Kevin Harwell: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_registrar.c: Fix named AOR and pjproject group deadlock.<br><br>One of the patches for ASTERISK_27147 introduced a deadlock regression.<br>When the connection oriented transport shut down, the code attempted to<br>remove the associated contact. However, that same transport had just<br>requested a registration that we hadn't responded to yet. Depending<br>upon timing we could deadlock.<br><br>* Made send the REGISTER response after we completed processing the<br>request contacts and released the named AOR lock to avoid the deadlock.<br><br>ASTERISK-27391<br><br>Change-Id: I89a90f87cb7a02facbafb44c75d8845f93417364<br>---<br>M res/res_pjsip_registrar.c<br>1 file changed, 39 insertions(+), 19 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c<br>index 8ba8597..cdf045f 100644<br>--- a/res/res_pjsip_registrar.c<br>+++ b/res/res_pjsip_registrar.c<br>@@ -447,11 +447,19 @@<br> AST_VECTOR_FREE(&contact_vec);<br> }<br> <br>-static int register_aor_core(pjsip_rx_data *rdata,<br>+struct aor_core_response {<br>+ /*! Tx data to use for statefull response. NULL for stateless response. */<br>+ pjsip_tx_data *tdata;<br>+ /*! SIP response code to send in stateless response */<br>+ int code;<br>+};<br>+<br>+static void register_aor_core(pjsip_rx_data *rdata,<br> struct ast_sip_endpoint *endpoint,<br> struct ast_sip_aor *aor,<br> const char *aor_name,<br>- struct ao2_container *contacts)<br>+ struct ao2_container *contacts,<br>+ struct aor_core_response *response)<br> {<br> static const pj_str_t USER_AGENT = { "User-Agent", 10 };<br> <br>@@ -480,19 +488,19 @@<br> <br> if (registrar_validate_contacts(rdata, contacts, aor, &added, &updated, &deleted)) {<br> /* The provided Contact headers do not conform to the specification */<br>- pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 400, NULL, NULL, NULL);<br> ast_sip_report_failed_acl(endpoint, rdata, "registrar_invalid_contacts_provided");<br> ast_log(LOG_WARNING, "Failed to validate contacts in REGISTER request from '%s'\n",<br> ast_sorcery_object_get_id(endpoint));<br>- return PJ_TRUE;<br>+ response->code = 400;<br>+ return;<br> }<br> <br> if (registrar_validate_path(rdata, aor, &path_str)) {<br> /* Ensure that intervening proxies did not make invalid modifications to the request */<br>- pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 420, NULL, NULL, NULL);<br> ast_log(LOG_WARNING, "Invalid modifications made to REGISTER request from '%s' by intervening proxy\n",<br> ast_sorcery_object_get_id(endpoint));<br>- return PJ_TRUE;<br>+ response->code = 420;<br>+ return;<br> }<br> <br> if (aor->remove_existing) {<br>@@ -504,18 +512,18 @@<br> }<br> if (contact_count > aor->max_contacts) {<br> /* Enforce the maximum number of contacts */<br>- pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 403, NULL, NULL, NULL);<br> ast_sip_report_failed_acl(endpoint, rdata, "registrar_attempt_exceeds_maximum_configured_contacts");<br> ast_log(LOG_WARNING, "Registration attempt from endpoint '%s' to AOR '%s' will exceed max contacts of %u\n",<br> ast_sorcery_object_get_id(endpoint), aor_name, aor->max_contacts);<br>- return PJ_TRUE;<br>+ response->code = 403;<br>+ return;<br> }<br> <br> details.pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(),<br> "Contact Comparison", 256, 256);<br> if (!details.pool) {<br>- pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);<br>- return PJ_TRUE;<br>+ response->code = 500;<br>+ return;<br> }<br> <br> user_agent_hdr = pjsip_msg_find_hdr_by_name(rdata->msg_info.msg, &USER_AGENT, NULL);<br>@@ -730,8 +738,8 @@<br> /* Re-retrieve contacts. Caller will clean up the original container. */<br> contacts = ast_sip_location_retrieve_aor_contacts_nolock(aor);<br> if (!contacts) {<br>- pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);<br>- return PJ_TRUE;<br>+ response->code = 500;<br>+ return;<br> }<br> response_contact = ao2_callback(contacts, 0, NULL, NULL);<br> <br>@@ -739,7 +747,8 @@<br> if (ast_sip_create_response(rdata, 200, response_contact, &tdata) != PJ_SUCCESS) {<br> ao2_cleanup(response_contact);<br> ao2_cleanup(contacts);<br>- return PJ_TRUE;<br>+ response->code = 500;<br>+ return;<br> }<br> ao2_cleanup(response_contact);<br> <br>@@ -754,9 +763,7 @@<br> pjsip_msg_add_hdr(tdata->msg, (pjsip_hdr*)expires_hdr);<br> }<br> <br>- ast_sip_send_stateful_response(rdata, tdata, endpoint);<br>-<br>- return PJ_TRUE;<br>+ response->tdata = tdata;<br> }<br> <br> static int register_aor(pjsip_rx_data *rdata,<br>@@ -764,12 +771,16 @@<br> struct ast_sip_aor *aor,<br> const char *aor_name)<br> {<br>- int res;<br>+ struct aor_core_response response = {<br>+ .code = 500,<br>+ };<br> struct ao2_container *contacts = NULL;<br> struct ast_named_lock *lock;<br> <br> lock = ast_named_lock_get(AST_NAMED_LOCK_TYPE_MUTEX, "aor", aor_name);<br> if (!lock) {<br>+ pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(),<br>+ rdata, response.code, NULL, NULL, NULL);<br> return PJ_TRUE;<br> }<br> <br>@@ -778,15 +789,24 @@<br> if (!contacts) {<br> ao2_unlock(lock);<br> ast_named_lock_put(lock);<br>+ pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(),<br>+ rdata, response.code, NULL, NULL, NULL);<br> return PJ_TRUE;<br> }<br> <br>- res = register_aor_core(rdata, endpoint, aor, aor_name, contacts);<br>+ register_aor_core(rdata, endpoint, aor, aor_name, contacts, &response);<br> ao2_cleanup(contacts);<br> ao2_unlock(lock);<br> ast_named_lock_put(lock);<br> <br>- return res;<br>+ /* Now send the REGISTER response to the peer */<br>+ if (response.tdata) {<br>+ ast_sip_send_stateful_response(rdata, response.tdata, endpoint);<br>+ } else {<br>+ pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(),<br>+ rdata, response.code, NULL, NULL, NULL);<br>+ }<br>+ return PJ_TRUE;<br> }<br> <br> static int match_aor(const char *aor_name, const char *id)<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7151">change 7151</a>. To unsubscribe, 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/7151"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13.18 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I89a90f87cb7a02facbafb44c75d8845f93417364 </div>
<div style="display:none"> Gerrit-Change-Number: 7151 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>