[Asterisk-code-review] sorcery: Create function ast sorcery lockable alloc. (asterisk[14])

[Corey Farrell]

Corey Farrell has uploaded a new change for review.

  https://gerrit.asterisk.org/3785

Change subject: sorcery: Create function ast_sorcery_lockable_alloc.
......................................................................

sorcery: Create function ast_sorcery_lockable_alloc.

Create an alternative to ast_sorcery_generic_alloc which uses astobj2
shared locking. Use this new method for the 'struct ast_sip_aor' allocator.

Change-Id: I3f62f2ada64b622571950278fbb6ad57395b5d6f
---
M include/asterisk/sorcery.h
M main/sorcery.c
M res/res_pjsip/location.c
M res/res_pjsip_registrar.c
4 files changed, 58 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/85/3785/1

diff --git a/include/asterisk/sorcery.h b/include/asterisk/sorcery.h
index 23219ec..8966338 100644
--- a/include/asterisk/sorcery.h
+++ b/include/asterisk/sorcery.h
@@ -988,10 +988,30 @@
  *
  * \retval non-NULL success
  * \retval NULL failure
+ *
+ * \note The returned object does not support AO2 locking.
  */
 void *ast_sorcery_generic_alloc(size_t size, ao2_destructor_fn destructor);
 
 /*!
+ * \since 14.1.0
+ * \brief Allocate a generic sorcery capable object with locking.
+ *
+ * \details Sorcery objects may be replaced with new allocations during reloads.
+ * If locking is required on sorcery objects it must be shared between the old
+ * object and the new one.  lockobj can be any AO2 object with locking enabled,
+ * but in most cases named locks should be used to provide stable locking.
+ *
+ * \param size Size of the object
+ * \param destructor Optional destructor function
+ * \param lockobj An AO2 object that will provide locking.
+ *
+ * \retval non-NULL success
+ * \retval NULL failure
+ */
+void *ast_sorcery_lockable_alloc(size_t size, ao2_destructor_fn destructor, void *lockobj);
+
+/*!
  * \brief Allocate an object
  *
  * \param sorcery Pointer to a sorcery structure
diff --git a/main/sorcery.c b/main/sorcery.c
index 55ee830..9f8c35c 100644
--- a/main/sorcery.c
+++ b/main/sorcery.c
@@ -1736,9 +1736,26 @@
 	ast_free(details->object->id);
 }
 
+void *ast_sorcery_lockable_alloc(size_t size, ao2_destructor_fn destructor, void *lockobj)
+{
+	void *object = ao2_alloc_with_lockobj(size + sizeof(struct ast_sorcery_object),
+		sorcery_object_destructor, lockobj, "");
+	struct ast_sorcery_object_details *details = object;
+
+	if (!object) {
+		return NULL;
+	}
+
+	details->object = object + size;
+	details->object->destructor = destructor;
+
+	return object;
+}
+
 void *ast_sorcery_generic_alloc(size_t size, ao2_destructor_fn destructor)
 {
-	void *object = ao2_alloc_options(size + sizeof(struct ast_sorcery_object), sorcery_object_destructor, AO2_ALLOC_OPT_LOCK_NOLOCK);
+	void *object = ao2_alloc_options(size + sizeof(struct ast_sorcery_object),
+		sorcery_object_destructor, AO2_ALLOC_OPT_LOCK_NOLOCK);
 	struct ast_sorcery_object_details *details = object;
 
 	if (!object) {
diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c
index 0ab69ec..5abfcab 100644
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -49,11 +49,22 @@
 /*! \brief Allocator for AOR */
 static void *aor_alloc(const char *name)
 {
-	struct ast_sip_aor *aor = ast_sorcery_generic_alloc(sizeof(struct ast_sip_aor), aor_destroy);
+	void *lock;
+	struct ast_sip_aor *aor;
+
+	lock = ast_named_lock_get(AST_NAMED_LOCK_TYPE_MUTEX, "aor", name);
+	if (!lock) {
+		return NULL;
+	}
+
+	aor = ast_sorcery_lockable_alloc(sizeof(struct ast_sip_aor), aor_destroy, lock);
+	ao2_ref(lock, -1);
+
 	if (!aor) {
 		return NULL;
 	}
 	ast_string_field_init(aor, 128);
+
 	return aor;
 }
 
@@ -206,17 +217,11 @@
 struct ao2_container *ast_sip_location_retrieve_aor_contacts(const struct ast_sip_aor *aor)
 {
 	struct ao2_container *contacts;
-	struct ast_named_lock *lock;
 
-	lock = ast_named_lock_get(AST_NAMED_LOCK_TYPE_MUTEX, "aor", ast_sorcery_object_get_id(aor));
-	if (!lock) {
-		return NULL;
-	}
-
-	ao2_lock(lock);
+	/* ao2_lock / ao2_unlock do not actually write aor since it has an ao2 lockobj. */
+	ao2_lock((void*)aor);
 	contacts = ast_sip_location_retrieve_aor_contacts_nolock(aor);
-	ao2_unlock(lock);
-	ast_named_lock_put(lock);
+	ao2_unlock((void*)aor);
 
 	return contacts;
 }
@@ -369,19 +374,12 @@
 		struct ast_sip_endpoint *endpoint)
 {
 	int res;
-	struct ast_named_lock *lock;
 
-	lock = ast_named_lock_get(AST_NAMED_LOCK_TYPE_MUTEX, "aor", ast_sorcery_object_get_id(aor));
-	if (!lock) {
-		return -1;
-	}
-
-	ao2_lock(lock);
+	ao2_lock(aor);
 	res = ast_sip_location_add_contact_nolock(aor, uri, expiration_time, path_info, user_agent,
 		via_addr, via_port, call_id,
 		endpoint);
-	ao2_unlock(lock);
-	ast_named_lock_put(lock);
+	ao2_unlock(aor);
 
 	return res;
 }
diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c
index f99a3b8..47d5991 100644
--- a/res/res_pjsip_registrar.c
+++ b/res/res_pjsip_registrar.c
@@ -523,25 +523,17 @@
 {
 	int res;
 	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) {
-		return PJ_TRUE;
-	}
-
-	ao2_lock(lock);
+	ao2_lock(aor);
 	contacts = ast_sip_location_retrieve_aor_contacts_nolock(aor);
 	if (!contacts) {
-		ao2_unlock(lock);
-		ast_named_lock_put(lock);
+		ao2_unlock(aor);
 		return PJ_TRUE;
 	}
 
 	res = register_aor_core(rdata, endpoint, aor, aor_name, contacts);
 	ao2_cleanup(contacts);
-	ao2_unlock(lock);
-	ast_named_lock_put(lock);
+	ao2_unlock(aor);
 
 	return res;
 }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3f62f2ada64b622571950278fbb6ad57395b5d6f
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Corey Farrell <git at cfware.com>