[Asterisk-code-review] Fix creation race of contact status structures. (asterisk[13])

[Mark Michelson]

Mark Michelson has uploaded a new change for review.

  https://gerrit.asterisk.org/2249

Change subject: Fix creation race of contact_status structures.
......................................................................

Fix creation race of contact_status structures.

It is possible when processing a SIP REGISTER request to have two
threads end up creating contact_status structures in sorcery.
contact_status is created using a "find or create" function. If two
threads call into this at the same time, each thread will fail to find
an existing contact_status, and so both will end up creating a new
contact status.

During testing, we would see sporadic failures because the
PJSIP_CONTACT() dialplan function would operate on a different
contact_status than what had been updated by res_pjsip/pjsip_options.

The fix here is two-fold:
1) The "find or create" function for contact_status now has a lock
around the entire operation. This way, if two threads attempt the
operation simultaneously, the first to get there will create the object,
and the second will find the object created by the first thread.

2) res_sorcery_memory has had its create callback updated so that it
will not allow for objects with duplicate IDs to be created.

Change-Id: I55b1460ff1eb0af0a3697b82d7c2bac9f6af5b97
---
M res/res_pjsip/pjsip_options.c
M res/res_sorcery_memory.c
2 files changed, 17 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/49/2249/1

diff --git a/res/res_pjsip/pjsip_options.c b/res/res_pjsip/pjsip_options.c
index 8b75a5f..f3c0737 100644
--- a/res/res_pjsip/pjsip_options.c
+++ b/res/res_pjsip/pjsip_options.c
@@ -105,6 +105,8 @@
 	return status;
 }
 
+AST_MUTEX_DEFINE_STATIC(creation_lock);
+
 /*!
  * \brief Retrieve a ast_sip_contact_status object from sorcery creating
  *        one if not found.
@@ -112,6 +114,7 @@
 struct ast_sip_contact_status *ast_res_pjsip_find_or_create_contact_status(const struct ast_sip_contact *contact)
 {
 	struct ast_sip_contact_status *status;
+	SCOPED_MUTEX(lock, &creation_lock);
 
 	status = ast_sorcery_retrieve_by_id(ast_sip_get_sorcery(), CONTACT_STATUS,
 		ast_sorcery_object_get_id(contact));
diff --git a/res/res_sorcery_memory.c b/res/res_sorcery_memory.c
index 7d398c2..19b687f 100644
--- a/res/res_sorcery_memory.c
+++ b/res/res_sorcery_memory.c
@@ -98,7 +98,21 @@
 
 static int sorcery_memory_create(const struct ast_sorcery *sorcery, void *data, void *object)
 {
+	void *existing;
+
+	ao2_lock(data);
+
+	existing = ao2_find(data, ast_sorcery_object_get_id(object), OBJ_KEY);
+	if (existing) {
+		ao2_ref(existing, -1);
+		ao2_unlock(data);
+		return -1;
+	}
+
 	ao2_link(data, object);
+
+	ao2_unlock(data);
+
 	return 0;
 }
 

-- 
To view, visit https://gerrit.asterisk.org/2249
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I55b1460ff1eb0af0a3697b82d7c2bac9f6af5b97
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>