[Asterisk-code-review] res pjsip registrar.c: Fix named AOR and pjproject group dea... (asterisk[13.18])
Richard Mudgett
asteriskteam at digium.com
Thu Nov 9 11:25:54 CST 2017
Richard Mudgett has uploaded this change for review. ( https://gerrit.asterisk.org/7151
Change subject: res_pjsip_registrar.c: Fix named AOR and pjproject group deadlock.
......................................................................
res_pjsip_registrar.c: Fix named AOR and pjproject group deadlock.
One of the patches for ASTERISK_27147 introduced a deadlock regression.
When the connection oriented transport shut down, the code attempted to
remove the associated contact. However, that same transport had just
requested a registration that we hadn't responded to yet. Depending
upon timing we could deadlock.
* Made send the REGISTER response after we completed processing the
request contacts and released the named AOR lock to avoid the deadlock.
ASTERISK-27391
Change-Id: I89a90f87cb7a02facbafb44c75d8845f93417364
---
M res/res_pjsip_registrar.c
1 file changed, 39 insertions(+), 19 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/51/7151/1
diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c
index 8ba8597..cdf045f 100644
--- a/res/res_pjsip_registrar.c
+++ b/res/res_pjsip_registrar.c
@@ -447,11 +447,19 @@
AST_VECTOR_FREE(&contact_vec);
}
-static int register_aor_core(pjsip_rx_data *rdata,
+struct aor_core_response {
+ /*! Tx data to use for statefull response. NULL for stateless response. */
+ pjsip_tx_data *tdata;
+ /*! SIP response code to send in stateless response */
+ int code;
+};
+
+static void register_aor_core(pjsip_rx_data *rdata,
struct ast_sip_endpoint *endpoint,
struct ast_sip_aor *aor,
const char *aor_name,
- struct ao2_container *contacts)
+ struct ao2_container *contacts,
+ struct aor_core_response *response)
{
static const pj_str_t USER_AGENT = { "User-Agent", 10 };
@@ -480,19 +488,19 @@
if (registrar_validate_contacts(rdata, contacts, aor, &added, &updated, &deleted)) {
/* The provided Contact headers do not conform to the specification */
- pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 400, NULL, NULL, NULL);
ast_sip_report_failed_acl(endpoint, rdata, "registrar_invalid_contacts_provided");
ast_log(LOG_WARNING, "Failed to validate contacts in REGISTER request from '%s'\n",
ast_sorcery_object_get_id(endpoint));
- return PJ_TRUE;
+ response->code = 400;
+ return;
}
if (registrar_validate_path(rdata, aor, &path_str)) {
/* Ensure that intervening proxies did not make invalid modifications to the request */
- pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 420, NULL, NULL, NULL);
ast_log(LOG_WARNING, "Invalid modifications made to REGISTER request from '%s' by intervening proxy\n",
ast_sorcery_object_get_id(endpoint));
- return PJ_TRUE;
+ response->code = 420;
+ return;
}
if (aor->remove_existing) {
@@ -504,18 +512,18 @@
}
if (contact_count > aor->max_contacts) {
/* Enforce the maximum number of contacts */
- pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 403, NULL, NULL, NULL);
ast_sip_report_failed_acl(endpoint, rdata, "registrar_attempt_exceeds_maximum_configured_contacts");
ast_log(LOG_WARNING, "Registration attempt from endpoint '%s' to AOR '%s' will exceed max contacts of %u\n",
ast_sorcery_object_get_id(endpoint), aor_name, aor->max_contacts);
- return PJ_TRUE;
+ response->code = 403;
+ return;
}
details.pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(),
"Contact Comparison", 256, 256);
if (!details.pool) {
- pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);
- return PJ_TRUE;
+ response->code = 500;
+ return;
}
user_agent_hdr = pjsip_msg_find_hdr_by_name(rdata->msg_info.msg, &USER_AGENT, NULL);
@@ -730,8 +738,8 @@
/* Re-retrieve contacts. Caller will clean up the original container. */
contacts = ast_sip_location_retrieve_aor_contacts_nolock(aor);
if (!contacts) {
- pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);
- return PJ_TRUE;
+ response->code = 500;
+ return;
}
response_contact = ao2_callback(contacts, 0, NULL, NULL);
@@ -739,7 +747,8 @@
if (ast_sip_create_response(rdata, 200, response_contact, &tdata) != PJ_SUCCESS) {
ao2_cleanup(response_contact);
ao2_cleanup(contacts);
- return PJ_TRUE;
+ response->code = 500;
+ return;
}
ao2_cleanup(response_contact);
@@ -754,9 +763,7 @@
pjsip_msg_add_hdr(tdata->msg, (pjsip_hdr*)expires_hdr);
}
- ast_sip_send_stateful_response(rdata, tdata, endpoint);
-
- return PJ_TRUE;
+ response->tdata = tdata;
}
static int register_aor(pjsip_rx_data *rdata,
@@ -764,12 +771,16 @@
struct ast_sip_aor *aor,
const char *aor_name)
{
- int res;
+ struct aor_core_response response = {
+ .code = 500,
+ };
struct ao2_container *contacts = NULL;
struct ast_named_lock *lock;
lock = ast_named_lock_get(AST_NAMED_LOCK_TYPE_MUTEX, "aor", aor_name);
if (!lock) {
+ pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(),
+ rdata, response.code, NULL, NULL, NULL);
return PJ_TRUE;
}
@@ -778,15 +789,24 @@
if (!contacts) {
ao2_unlock(lock);
ast_named_lock_put(lock);
+ pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(),
+ rdata, response.code, NULL, NULL, NULL);
return PJ_TRUE;
}
- res = register_aor_core(rdata, endpoint, aor, aor_name, contacts);
+ register_aor_core(rdata, endpoint, aor, aor_name, contacts, &response);
ao2_cleanup(contacts);
ao2_unlock(lock);
ast_named_lock_put(lock);
- return res;
+ /* Now send the REGISTER response to the peer */
+ if (response.tdata) {
+ ast_sip_send_stateful_response(rdata, response.tdata, endpoint);
+ } else {
+ pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(),
+ rdata, response.code, NULL, NULL, NULL);
+ }
+ return PJ_TRUE;
}
static int match_aor(const char *aor_name, const char *id)
--
To view, visit https://gerrit.asterisk.org/7151
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 13.18
Gerrit-MessageType: newchange
Gerrit-Change-Id: I89a90f87cb7a02facbafb44c75d8845f93417364
Gerrit-Change-Number: 7151
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171109/374d6d27/attachment.html>
More information about the asterisk-code-review
mailing list