[Asterisk-code-review] lock: Add named lock capability (asterisk[master])

Joshua Colp asteriskteam at digium.com
Mon Apr 11 12:58:45 CDT 2016


Joshua Colp has submitted this change and it was merged.

Change subject: lock:  Add named lock capability
......................................................................


lock:  Add named lock capability

Locking some objects like sorcery objects can be tricky because the underlying
ao2 object may not be the same for all callers.  For instance, two threads that
call ast_sorcery_retrieve_by_id on the same aor name might actually get 2
different ao2 objects if the underlying wizard had to rehydrate the aor from a
database. Locking one ao2 object doesn't have any effect on the other even if
those objects had locks in the first place.

Named locks allow access control by keyspace and key strings.  Now an "aor"
named "1000" can be locked and any other thread attempting to lock "aor" "1000"
will wait regardless of whether the underlying ao2 object is the same or not.
Mutex and rwlocks are supported.

This capability will initially be used to lock an aor when multiple threads may
be attempting to prune expired contacts from it.

Change-Id: If258c0b7f92b02d07243ce70e535821a1ea7fb45
---
M include/asterisk/_private.h
A include/asterisk/named_locks.h
M main/asterisk.c
A main/named_locks.c
A tests/test_named_lock.c
5 files changed, 404 insertions(+), 0 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/include/asterisk/_private.h b/include/asterisk/_private.h
index a4a4f1b..6dbf24f 100644
--- a/include/asterisk/_private.h
+++ b/include/asterisk/_private.h
@@ -39,6 +39,7 @@
 void threadstorage_init(void);		/*!< Provided by threadstorage.c */
 int ast_device_state_engine_init(void);	/*!< Provided by devicestate.c */
 int astobj2_init(void);			/*!< Provided by astobj2.c */
+int ast_named_locks_init(void);		/*!< Provided by named_locks.c */
 int ast_file_init(void);		/*!< Provided by file.c */
 int ast_features_init(void);            /*!< Provided by features.c */
 void ast_autoservice_init(void);	/*!< Provided by autoservice.c */
diff --git a/include/asterisk/named_locks.h b/include/asterisk/named_locks.h
new file mode 100644
index 0000000..0fe07d9
--- /dev/null
+++ b/include/asterisk/named_locks.h
@@ -0,0 +1,105 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2016, Fairview 5 Engineering, LLC
+ *
+ * George Joseph <george.joseph at fairview5.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*! \file
+ *
+ * \brief Named Locks
+ *
+ * \author George Joseph <george.joseph at fairview5.com>
+ */
+
+#ifndef INCLUDE_ASTERISK_NAMED_LOCKS_H_
+#define INCLUDE_ASTERISK_NAMED_LOCKS_H_
+
+#include "asterisk/astobj2.h"
+
+/*!
+ * \defgroup named_locks Named mutex and read-write locks
+ * @{
+ * \page NamedLocks Named mutex and read-write locks
+ * \since 13.9.0
+ *
+ * Locking some objects like sorcery objects can be tricky because the underlying
+ * ao2 object may not be the same for all callers.  For instance, two threads that
+ * call ast_sorcery_retrieve_by_id on the same aor name might actually get 2 different
+ * ao2 objects if the underlying wizard had to rehydrate the aor from a database.
+ * Locking one ao2 object doesn't have any effect on the other even if those objects
+ * had locks in the first place
+ *
+ * Named locks allow access control by name.  Now an aor named "1000" can be locked and
+ * any other thread attempting to lock the aor named "1000" will wait regardless of whether
+ * the underlying ao2 object is the same or not.
+ *
+ * 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.
+ */
+
+/*!
+ * \brief Which type of lock to request.
+ */
+enum ast_named_lock_type {
+	/*! Request a named mutex. */
+	AST_NAMED_LOCK_TYPE_MUTEX = AO2_ALLOC_OPT_LOCK_MUTEX,
+	/*! Request a named read/write lock. */
+	AST_NAMED_LOCK_TYPE_RWLOCK = AO2_ALLOC_OPT_LOCK_RWLOCK,
+};
+
+struct ast_named_lock;
+
+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
+ * \since 13.9.0
+ *
+ * \param lock_type One of ast_named_lock_type
+ * \param keyspace
+ * \param key
+ * \retval A pointer to an ast_named_lock structure
+ * \retval NULL on error
+ *
+ * \note
+ * keyspace and key can be anything.  For sorcery objects, keyspace could be the object type
+ * and key could be the object id.
+ */
+#define ast_named_lock_get(lock_type, keyspace, key) \
+	__ast_named_lock_get(__FILE__, __LINE__, __PRETTY_FUNCTION__, lock_type, \
+		keyspace, key)
+
+/*!
+ * \brief Put a named lock handle away
+ * \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)
+
+/*!
+ * @}
+ */
+
+#endif /* INCLUDE_ASTERISK_NAMED_LOCKS_H_ */
diff --git a/main/asterisk.c b/main/asterisk.c
index 7636ec7..bf2206c 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -4339,6 +4339,11 @@
 		exit(1);
 	}
 
+	if (ast_named_locks_init()) {
+		printf("Failed: ast_named_locks_init\n%s", term_quit());
+		exit(1);
+	}
+
 	if (ast_opt_console) {
 		if (el_hist == NULL || el == NULL)
 			ast_el_initialize();
diff --git a/main/named_locks.c b/main/named_locks.c
new file mode 100644
index 0000000..b977b55
--- /dev/null
+++ b/main/named_locks.c
@@ -0,0 +1,142 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2016, Fairview 5 Engineering, LLC
+ *
+ * George Joseph <george.joseph at fairview5.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*! \file
+ *
+ * \brief Named Locks
+ *
+ * \author George Joseph <george.joseph at fairview5.com>
+ */
+
+#include "asterisk.h"
+
+ASTERISK_REGISTER_FILE()
+
+#include "asterisk/_private.h"
+#include "asterisk/astobj2.h"
+#include "asterisk/named_locks.h"
+#include "asterisk/utils.h"
+
+struct ao2_container *named_locks;
+#define NAMED_LOCKS_BUCKETS 101
+
+struct ast_named_lock {
+	char key[0];
+};
+
+static int named_locks_hash(const void *obj, const int flags)
+{
+	const struct ast_named_lock *lock = obj;
+
+	switch (flags & OBJ_SEARCH_MASK) {
+	case OBJ_SEARCH_KEY:
+		return ast_str_hash(obj);
+	case OBJ_SEARCH_OBJECT:
+		return ast_str_hash(lock->key);
+	default:
+		/* Hash can only work on something with a full key. */
+		ast_assert(0);
+		return 0;
+	}
+}
+
+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 char *right_key = obj_right;
+	int cmp;
+
+	switch (flags & OBJ_SEARCH_MASK) {
+	case OBJ_SEARCH_OBJECT:
+		right_key = object_right->key;
+		/* Fall through */
+	case OBJ_SEARCH_KEY:
+		cmp = strcmp(object_left->key, right_key);
+		break;
+	case OBJ_SEARCH_PARTIAL_KEY:
+		cmp = strncmp(object_left->key, right_key, strlen(right_key));
+		break;
+	default:
+		cmp = 0;
+		break;
+	}
+
+	return cmp ? 0 : CMP_MATCH;
+}
+
+static void named_locks_shutdown(void)
+{
+	ao2_cleanup(named_locks);
+}
+
+int ast_named_locks_init(void)
+{
+	named_locks = ao2_container_alloc_hash(0, 0, NAMED_LOCKS_BUCKETS, named_locks_hash, NULL,
+		named_locks_cmp);
+	if (!named_locks) {
+		return -1;
+	}
+
+	ast_register_cleanup(named_locks_shutdown);
+
+	return 0;
+}
+
+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 ast_named_lock *lock = NULL;
+	int concat_key_buff_len = strlen(keyspace) + strlen(key) + 2;
+	char *concat_key = ast_alloca(concat_key_buff_len);
+
+	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) {
+		ao2_unlock(named_locks);
+		ast_assert((ao2_options_get(lock) & AO2_ALLOC_OPT_LOCK_MASK) == lock_type);
+		return lock;
+	}
+
+	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);
+	}
+	ao2_unlock(named_locks);
+
+	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);
+	}
+	ao2_unlock(named_locks);
+
+	return 0;
+}
diff --git a/tests/test_named_lock.c b/tests/test_named_lock.c
new file mode 100644
index 0000000..9e38308
--- /dev/null
+++ b/tests/test_named_lock.c
@@ -0,0 +1,151 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2016, Fairview 5 Engineering, LLC
+ *
+ * George Joseph <george.joseph at fairview5.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*!
+ * \file
+ * \brief Named Lock unit tests
+ *
+ * \author George Joseph <george.joseph at fairview5.com>
+ *
+ */
+
+/*** MODULEINFO
+	<depend>TEST_FRAMEWORK</depend>
+	<support_level>core</support_level>
+ ***/
+
+#include "asterisk.h"
+#include <signal.h>
+
+#include "asterisk/test.h"
+#include "asterisk/utils.h"
+#include "asterisk/module.h"
+#include "asterisk/lock.h"
+#include "asterisk/named_locks.h"
+
+static void *lock_thread(void *data)
+{
+	struct ast_named_lock *lock = ast_named_lock_get(AST_NAMED_LOCK_TYPE_MUTEX, "lock_test", data);
+
+	if (!lock) {
+		return NULL;
+	}
+
+	ao2_lock(lock);
+	usleep(3000000);
+	ao2_unlock(lock);
+
+	ast_named_lock_put(lock);
+
+	return NULL;
+}
+
+AST_TEST_DEFINE(named_lock_test)
+{
+	enum ast_test_result_state res = AST_TEST_FAIL;
+	struct ast_named_lock *lock1 = NULL;
+	struct ast_named_lock *lock2 = NULL;
+	pthread_t thread1;
+	pthread_t thread2;
+	struct timeval start_time;
+	int64_t duration;
+
+	switch(cmd) {
+	case TEST_INIT:
+		info->name = "named_lock_test";
+		info->category = "/main/lock/";
+		info->summary = "Named Lock test";
+		info->description =
+			"Tests that named locks operate as expected";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	ast_test_status_update(test, "This test should take about 3 seconds\n");
+
+	/* 2 locks/threads to make sure they're independent */
+	ast_pthread_create(&thread1, NULL, lock_thread, "lock_1");
+	ast_pthread_create(&thread2, NULL, lock_thread, "lock_2");
+
+	lock1 = ast_named_lock_get(AST_NAMED_LOCK_TYPE_MUTEX, "lock_test", "lock_1");
+	ast_test_validate_cleanup(test, lock1 != NULL, res, fail);
+
+	lock2 = ast_named_lock_get(AST_NAMED_LOCK_TYPE_MUTEX, "lock_test", "lock_2");
+	ast_test_validate_cleanup(test, lock2 != NULL, res, fail);
+
+	usleep(1000000);
+
+	/* These should both fail */
+	if (!ao2_trylock(lock1)) {
+		ast_test_status_update(test, "ao2_trylock on lock1 succeeded when it should have failed\n");
+		ao2_unlock(lock1);
+		goto fail;
+	}
+
+	if (!ao2_trylock(lock2)) {
+		ast_test_status_update(test, "ao2_trylock on lock2 succeeded when it should have failed\n");
+		ao2_unlock(lock2);
+		goto fail;
+	}
+
+	start_time = ast_tvnow();
+
+	/* These should both succeed eventually */
+	if (ao2_lock(lock1)) {
+		ast_test_status_update(test, "ao2_lock on lock1 failed\n");
+		goto fail;
+	}
+	ao2_unlock(lock1);
+
+	if (ao2_lock(lock2)) {
+		ast_test_status_update(test, "ao2_lock on lock2 failed\n");
+		goto fail;
+	}
+	ao2_unlock(lock2);
+
+	duration = ast_tvdiff_ms(ast_tvnow(), start_time);
+	ast_test_validate_cleanup(test, duration > 1500 && duration < 3500, res, fail);
+
+	res = AST_TEST_PASS;
+
+fail:
+
+	ast_named_lock_put(lock1);
+	ast_named_lock_put(lock2);
+
+	pthread_join(thread1, NULL);
+	pthread_join(thread2, NULL);
+
+	return res;
+}
+
+
+static int unload_module(void)
+{
+	AST_TEST_UNREGISTER(named_lock_test);
+	return 0;
+}
+
+static int load_module(void)
+{
+	AST_TEST_REGISTER(named_lock_test);
+	return AST_MODULE_LOAD_SUCCESS;
+}
+
+AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Named Lock test module");

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

Gerrit-MessageType: merged
Gerrit-Change-Id: If258c0b7f92b02d07243ce70e535821a1ea7fb45
Gerrit-PatchSet: 10
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>



More information about the asterisk-code-review mailing list