[asterisk-commits] named locks: Use ao2 weakproxy to deal with cleanup from con... (asterisk[master])

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Sep 6 11:20:57 CDT 2016


Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: named_locks: Use ao2_weakproxy to deal with cleanup from container.
......................................................................


named_locks: Use ao2_weakproxy to deal with cleanup from container.

This allows standard ao2 functions to be used to release references to
an ast_named_lock.  This change can cause less frequent locking of the
global named_locks container.  The container is no longer locked when a
named_lock reference is being release except when this causes the
named_lock to be destroyed.

Change-Id: I644e39c6d83a153d71b3fae77ec05599d725e7e6
---
M include/asterisk/named_locks.h
M main/named_locks.c
2 files changed, 60 insertions(+), 35 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/include/asterisk/named_locks.h b/include/asterisk/named_locks.h
index 0fe07d9..1959841 100644
--- a/include/asterisk/named_locks.h
+++ b/include/asterisk/named_locks.h
@@ -48,7 +48,7 @@
  * To use a named lock:
  * 	Call ast_named_lock_get with the appropriate keyspace and key.
  * 	Use the standard ao2 lock/unlock functions as needed.
- * 	Call ast_named_lock_put when you're finished with it.
+ * 	Call ao2_cleanup when you're finished with it.
  */
 
 /*!
@@ -65,9 +65,6 @@
 
 struct ast_named_lock *__ast_named_lock_get(const char *filename, int lineno, const char *func,
 	enum ast_named_lock_type lock_type, const char *keyspace, const char *key);
-
-int __ast_named_lock_put(const char *filename, int lineno, const char *func,
-	struct ast_named_lock *lock);
 
 /*!
  * \brief Geta named lock handle
@@ -92,11 +89,8 @@
  * \since 13.9.0
  *
  * \param lock The pointer to the ast_named_lock structure returned by ast_named_lock_get
- * \retval 0 Success
- * \retval -1 Failure
  */
-#define ast_named_lock_put(lock) \
-	__ast_named_lock_put(__FILE__, __LINE__, __PRETTY_FUNCTION__, lock)
+#define ast_named_lock_put(lock) ao2_cleanup(lock)
 
 /*!
  * @}
diff --git a/main/named_locks.c b/main/named_locks.c
index 5960483..c71f3b5 100644
--- a/main/named_locks.c
+++ b/main/named_locks.c
@@ -35,13 +35,17 @@
 struct ao2_container *named_locks;
 #define NAMED_LOCKS_BUCKETS 101
 
-struct ast_named_lock {
+struct named_lock_proxy {
+	AO2_WEAKPROXY();
 	char key[0];
+};
+
+struct ast_named_lock {
 };
 
 static int named_locks_hash(const void *obj, const int flags)
 {
-	const struct ast_named_lock *lock = obj;
+	const struct named_lock_proxy *lock = obj;
 
 	switch (flags & OBJ_SEARCH_MASK) {
 	case OBJ_SEARCH_KEY:
@@ -57,8 +61,8 @@
 
 static int named_locks_cmp(void *obj_left, void *obj_right, int flags)
 {
-	const struct ast_named_lock *object_left = obj_left;
-	const struct ast_named_lock *object_right = obj_right;
+	const struct named_lock_proxy *object_left = obj_left;
+	const struct named_lock_proxy *object_right = obj_right;
 	const char *right_key = obj_right;
 	int cmp;
 
@@ -98,45 +102,72 @@
 	return 0;
 }
 
+static void named_lock_proxy_cb(void *weakproxy, void *data)
+{
+	ao2_unlink(named_locks, weakproxy);
+}
+
 struct ast_named_lock *__ast_named_lock_get(const char *filename, int lineno, const char *func,
 	enum ast_named_lock_type lock_type, const char *keyspace, const char *key)
 {
+	struct named_lock_proxy *proxy = NULL;
 	struct ast_named_lock *lock = NULL;
-	int concat_key_buff_len = strlen(keyspace) + strlen(key) + 2;
-	char *concat_key = ast_alloca(concat_key_buff_len);
+	int keylen = strlen(keyspace) + strlen(key) + 2;
+	char *concat_key = ast_alloca(keylen);
 
 	sprintf(concat_key, "%s-%s", keyspace, key); /* Safe */
 
 	ao2_lock(named_locks);
-	lock = ao2_find(named_locks, concat_key, OBJ_SEARCH_KEY | OBJ_NOLOCK);
-	if (lock) {
+	proxy = ao2_find(named_locks, concat_key, OBJ_SEARCH_KEY | OBJ_NOLOCK);
+	if (proxy) {
 		ao2_unlock(named_locks);
-		ast_assert((ao2_options_get(lock) & AO2_ALLOC_OPT_LOCK_MASK) == lock_type);
-		return lock;
+		lock = __ao2_weakproxy_get_object(proxy, 0, __PRETTY_FUNCTION__, filename, lineno, func);
+
+		if (lock) {
+			/* We have an existing lock and it's not being destroyed. */
+			ao2_ref(proxy, -1);
+			ast_assert((ao2_options_get(lock) & AO2_ALLOC_OPT_LOCK_MASK) == lock_type);
+
+			return lock;
+		}
+
+		/* the old proxy is being destroyed, clean list before creating/adding new one */
+		ao2_lock(named_locks);
+		ao2_unlink_flags(named_locks, proxy, OBJ_NOLOCK);
+		ao2_ref(proxy, -1);
 	}
 
-	lock = ao2_alloc_options(sizeof(*lock) + concat_key_buff_len, NULL, lock_type);
-	if (lock) {
-		strcpy(lock->key, concat_key); /* Safe */
-		ao2_link_flags(named_locks, lock, OBJ_NOLOCK);
+	proxy = ao2_t_weakproxy_alloc(sizeof(*proxy) + keylen, NULL, concat_key);
+	if (!proxy) {
+		goto failure_cleanup;
 	}
+
+	lock = __ao2_alloc(sizeof(*lock) + keylen, NULL, lock_type, concat_key, filename, lineno, func);
+	if (!lock) {
+		goto failure_cleanup;
+	}
+
+	/* We have exclusive access to proxy and lock, no need for locking here. */
+	if (ao2_weakproxy_set_object(proxy, lock, OBJ_NOLOCK)) {
+		goto failure_cleanup;
+	}
+
+	if (ao2_weakproxy_subscribe(proxy, named_lock_proxy_cb, NULL, OBJ_NOLOCK)) {
+		goto failure_cleanup;
+	}
+
+	strcpy(proxy->key, concat_key); /* Safe */
+	ao2_link_flags(named_locks, proxy, OBJ_NOLOCK);
 	ao2_unlock(named_locks);
+	ao2_t_ref(proxy, -1, "Release allocation reference");
 
 	return lock;
-}
 
-int __ast_named_lock_put(const char *filename, int lineno, const char *func,
-	struct ast_named_lock *lock)
-{
-	if (!lock) {
-		return -1;
-	}
-
-	ao2_lock(named_locks);
-	if (ao2_ref(lock, -1) == 2) {
-		ao2_unlink_flags(named_locks, lock, OBJ_NOLOCK);
-	}
+failure_cleanup:
 	ao2_unlock(named_locks);
 
-	return 0;
+	ao2_cleanup(proxy);
+	ao2_cleanup(lock);
+
+	return NULL;
 }

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I644e39c6d83a153d71b3fae77ec05599d725e7e6
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>



More information about the asterisk-commits mailing list