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

George Joseph asteriskteam at digium.com
Fri Apr 1 12:54:27 CDT 2016


George Joseph has uploaded a new change for review.

  https://gerrit.asterisk.org/2522

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.

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
M include/asterisk/lock.h
M main/asterisk.c
M main/lock.c
A tests/test_named_lock.c
5 files changed, 384 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/22/2522/1

diff --git a/include/asterisk/_private.h b/include/asterisk/_private.h
index a4a4f1b..143d744 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_lock_init(void);			/*!< Provided by lock.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/lock.h b/include/asterisk/lock.h
index 35a244b..1d96683 100644
--- a/include/asterisk/lock.h
+++ b/include/asterisk/lock.h
@@ -751,4 +751,96 @@
 })
 #endif
 
+/*!
+ * \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.
+ *
+ * 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.
+ */
+
+/*!
+ * \brief Which lock to request.
+ * \internal
+ */
+enum ast_named_lock_req {
+	/*! Request the mutex lock be acquired. */
+	AST_NAMED_LOCK_REQ_MUTEX,
+	/*! Request the read lock be acquired. */
+	AST_NAMED_LOCK_REQ_RDLOCK,
+	/*! Request the write lock be acquired. */
+	AST_NAMED_LOCK_REQ_WRLOCK,
+};
+
+struct ast_named_lock;
+struct ast_named_lock *__ast_named_lock_lock(const char *filename, int lineno, const char *func,
+	enum ast_named_lock_req lock_how, const char *keyspace, const char *key);
+struct ast_named_lock *__ast_named_lock_trylock(const char *filename, int lineno, const char *func,
+	enum ast_named_lock_req lock_how, const char *keyspace, const char *key);
+
+/*!
+ * \brief Create and/or lock by name
+ * \since 13.9.0
+ *
+ * \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_lock(keyspace, key) \
+	__ast_named_lock_lock(__FILE__, __LINE__, __PRETTY_FUNCTION__, AST_NAMED_LOCK_REQ_MUTEX, keyspace, key)
+#define ast_named_lock_rdlock(keyspace, key) \
+	__ast_named_lock_lock(__FILE__, __LINE__, __PRETTY_FUNCTION__, AST_NAMED_LOCK_REQ_RDLOCK, keyspace, key)
+#define ast_named_lock_wrlock(keyspace, key) \
+	__ast_named_lock_lock(__FILE__, __LINE__, __PRETTY_FUNCTION__, AST_NAMED_LOCK_REQ_WRLOCK, keyspace, key)
+
+/*!
+ * \brief Try to create and/or lock by name (don't block if fail)
+ * \since 13.9.0
+ *
+ * \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_trylock(keyspace, key) \
+	__ast_named_lock_trylock(__FILE__, __LINE__, __PRETTY_FUNCTION__, AST_NAMED_LOCK_REQ_MUTEX, keyspace, key)
+#define ast_named_lock_tryrdlock(keyspace, key) \
+	__ast_named_lock_trylock(__FILE__, __LINE__, __PRETTY_FUNCTION__, AST_NAMED_LOCK_REQ_RDLOCK, keyspace, key)
+#define ast_named_lock_trywrlock(keyspace, key) \
+	__ast_named_lock_trylock(__FILE__, __LINE__, __PRETTY_FUNCTION__, AST_NAMED_LOCK_REQ_WRLOCK, keyspace, key)
+
+/*!
+ * \brief Unlock a named lock
+ * \since 13.9.0
+ *
+ * \param lock The pointer to the ast_named_lock structure returned by one of the lock functions
+ * \retval 0 Success
+ * \retval -1 Failure
+ */
+#define ast_named_lock_unlock(lock) \
+	__ast_named_lock_unlock(__FILE__, __LINE__, __PRETTY_FUNCTION__, lock)
+int __ast_named_lock_unlock(const char *filename, int lineno, const char *func, struct ast_named_lock *lock);
+
+/*!
+ * @}
+ */
+
 #endif /* _ASTERISK_LOCK_H */
diff --git a/main/asterisk.c b/main/asterisk.c
index 7636ec7..d607e62 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -4339,6 +4339,11 @@
 		exit(1);
 	}
 
+	if (ast_lock_init()) {
+		printf("Failed: ast_lock_init\n%s", term_quit());
+		exit(1);
+	}
+
 	if (ast_opt_console) {
 		if (el_hist == NULL || el == NULL)
 			ast_el_initialize();
diff --git a/main/lock.c b/main/lock.c
index 13b8fb3..2bcaeaf 100644
--- a/main/lock.c
+++ b/main/lock.c
@@ -31,6 +31,7 @@
 
 #include "asterisk/utils.h"
 #include "asterisk/lock.h"
+#include "asterisk/_private.h"
 
 /* Allow direct use of pthread_mutex_* / pthread_cond_* */
 #undef pthread_mutex_init
@@ -1357,3 +1358,161 @@
 
 	return res;
 }
+
+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;
+}
+
+int ast_lock_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;
+	}
+
+	return 0;
+}
+
+struct ast_named_lock *__ast_named_lock_lock(const char *filename, int lineno, const char *func,
+	enum ast_named_lock_req lock_how, const char *keyspace, const char *key)
+{
+	struct ast_named_lock *lock = NULL;
+	char *concat_key = ast_alloca(strlen(keyspace) + strlen(key) + 2);
+	int res = 0;
+
+	sprintf(concat_key, "%s-%s", keyspace, key);
+
+	ao2_lock(named_locks);
+	lock = ao2_find(named_locks, concat_key, OBJ_SEARCH_KEY | OBJ_NOLOCK);
+	if (!lock) {
+		lock = ao2_alloc_options(sizeof(*lock) + strlen(concat_key) + 1, NULL,
+			lock_how == AST_NAMED_LOCK_REQ_MUTEX ? AO2_ALLOC_OPT_LOCK_MUTEX : AO2_ALLOC_OPT_LOCK_RWLOCK);
+		strcpy(lock->key, concat_key); /* Safe */
+		ao2_link_flags(named_locks, lock, OBJ_NOLOCK);
+	}
+	ao2_unlock(named_locks);
+
+	switch (lock_how) {
+	case AST_NAMED_LOCK_REQ_MUTEX:
+		res = __ao2_lock(lock, AO2_LOCK_REQ_MUTEX, filename, func, lineno, concat_key);
+		break;
+	case AST_NAMED_LOCK_REQ_WRLOCK:
+		res = __ao2_lock(lock, AO2_LOCK_REQ_WRLOCK, filename, func, lineno, concat_key);
+		break;
+	case AST_NAMED_LOCK_REQ_RDLOCK:
+		res = __ao2_lock(lock, AO2_LOCK_REQ_RDLOCK, filename, func, lineno, concat_key);
+		break;
+	}
+
+	if (res) {
+		ao2_ref(lock, -1);
+		if (ao2_ref(lock, 0) == 1) {
+			ao2_unlink(named_locks, lock);
+		}
+		return NULL;
+	}
+
+	return lock;
+}
+
+struct ast_named_lock *__ast_named_lock_trylock(const char *filename, int lineno, const char *func,
+	enum ast_named_lock_req lock_how, const char *keyspace, const char *key)
+{
+	struct ast_named_lock *lock = NULL;
+	char *concat_key = ast_alloca(strlen(keyspace) + strlen(key) + 2);
+	int res = 0;
+
+	sprintf(concat_key, "%s-%s", keyspace, key);
+
+	ao2_lock(named_locks);
+	lock = ao2_find(named_locks, concat_key, OBJ_SEARCH_KEY | OBJ_NOLOCK);
+	if (!lock) {
+		lock = ao2_alloc_options(sizeof(*lock) + strlen(concat_key) + 1, NULL,
+			lock_how == AST_NAMED_LOCK_REQ_MUTEX ? AO2_ALLOC_OPT_LOCK_MUTEX : AO2_ALLOC_OPT_LOCK_RWLOCK);
+		strcpy(lock->key, concat_key); /* Safe */
+		ao2_link_flags(named_locks, lock, OBJ_NOLOCK);
+	}
+	ao2_unlock(named_locks);
+
+	switch (lock_how) {
+	case AST_NAMED_LOCK_REQ_MUTEX:
+		res = __ao2_trylock(lock, AO2_LOCK_REQ_MUTEX, filename, func, lineno, concat_key);
+		break;
+	case AST_NAMED_LOCK_REQ_WRLOCK:
+		res = __ao2_trylock(lock, AO2_LOCK_REQ_WRLOCK, filename, func, lineno, concat_key);
+		break;
+	case AST_NAMED_LOCK_REQ_RDLOCK:
+		res = __ao2_trylock(lock, AO2_LOCK_REQ_RDLOCK, filename, func, lineno, concat_key);
+		break;
+	}
+
+	if (res) {
+		ao2_ref(lock, -1);
+		if (ao2_ref(lock, 0) == 1) {
+			ao2_unlink(named_locks, lock);
+		}
+		return NULL;
+	}
+
+	return lock;
+}
+
+int __ast_named_lock_unlock(const char *filename, int lineno, const char *func, struct ast_named_lock *lock)
+{
+	if (!lock || __ao2_unlock(lock, filename, func, lineno, lock->key)) {
+		return -1;
+	}
+
+	ao2_lock(named_locks);
+	ao2_ref(lock, -1);
+	if (ao2_ref(lock, 0) == 1) {
+		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..3f1b4a7
--- /dev/null
+++ b/tests/test_named_lock.c
@@ -0,0 +1,127 @@
+/*
+ * 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 "asterisk/test.h"
+#include "asterisk/utils.h"
+#include "asterisk/module.h"
+#include "asterisk/lock.h"
+
+static void *lock_thread(void *data)
+{
+	struct ast_named_lock *lock;
+
+	lock = ast_named_lock_lock("lock_test", (const char *)data);
+	usleep(3000000);
+	ast_named_lock_unlock(lock);
+
+	return NULL;
+}
+
+AST_TEST_DEFINE(named_lock_test)
+{
+	enum ast_test_result_state res;
+	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");
+
+	usleep(1000000);
+	lock1 = ast_named_lock_trylock("lock_test", "lock_1");
+	ast_test_validate_cleanup(test, lock1 == NULL, res, fail);
+
+	lock2 = ast_named_lock_trylock("lock_test", "lock_2");
+	ast_test_validate_cleanup(test, lock2 == NULL, res, fail);
+
+	start_time = ast_tvnow();
+
+	lock1 = ast_named_lock_lock("lock_test", "lock_1");
+	ast_test_validate_cleanup(test, lock1 != NULL, res, fail);
+
+	lock2 = ast_named_lock_lock("lock_test", "lock_2");
+	ast_test_validate_cleanup(test, lock2 != NULL, res, fail);
+
+	ast_named_lock_unlock(lock1);
+	ast_named_lock_unlock(lock2);
+
+	duration = ast_tvdiff_ms(ast_tvnow(), start_time);
+	if (duration < 1500 || duration > 3500) {
+		ast_test_status_update(test, "Lock didn't take the correct time\n");
+		return AST_TEST_FAIL;
+	}
+
+	return AST_TEST_PASS;
+
+fail:
+
+	ast_named_lock_unlock(lock1);
+	ast_named_lock_unlock(lock2);
+	pthread_kill(thread1, SIGURG);
+	pthread_kill(thread2, SIGURG);
+
+	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: newchange
Gerrit-Change-Id: If258c0b7f92b02d07243ce70e535821a1ea7fb45
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>



More information about the asterisk-code-review mailing list