Merge trunk revision 2963:

Fix OBJDB locking

Patch fixes following situation:

1. objdb receives reload notification and ends in function
object_reload_config. This will call objdb_wrlock. I will call this
thread #1

2. Another thread will decide to update corosync statistics and calls
object_key_increment. This calls objdb_rdlock. This thread is #2. But
because condition (lock_thread != pthread_self()) is satisfied, it will
also calls pthread_rwlock_rdlock. This will blocks, because thread #1
holds the lock.

3. object_reload_config will call reload functions (as real example
xml2objdb). xml2objdb needs to calls object_create. This calls
objdb_rdlock, but will hang on pthread_mutex_lock(&meta_lock), because
this lock is held by thread #2.

-> deadlock

It is handled by using recursive mutex.

Also every function is now really locked.



git-svn-id: http://svn.fedorahosted.org/svn/corosync/branches/flatiron@2977 fd59a12c-fef9-0310-b244-a6a79926bd2f

exec/objdb.c

@@ -33,11 +33,15 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#define _XOPEN_SOURCE 600
+
 #include <config.h>
 
 #include <stdio.h>
 #include <errno.h>
 
+#include <pthread.h>
+
 #include <corosync/list.h>
 #include <corosync/hdb.h>
 #include <corosync/lcr/lcr_comp.h>
@@ -97,44 +101,21 @@ struct object_find_instance {
 
 struct objdb_iface_ver0 objdb_iface;
 struct list_head objdb_trackers_head;
-static pthread_rwlock_t reload_lock;
-static pthread_t lock_thread;
-static pthread_mutex_t meta_lock;
+static pthread_mutex_t objdb_mutex;
+static pthread_mutexattr_t objdb_mutex_attr;
 
 DECLARE_HDB_DATABASE (object_instance_database,NULL);
 
 DECLARE_HDB_DATABASE (object_find_instance_database,NULL);
 
-static void objdb_wrlock(void)
-{
-	pthread_mutex_lock(&meta_lock);
-	pthread_rwlock_wrlock(&reload_lock);
-	lock_thread = pthread_self();
-	pthread_mutex_unlock(&meta_lock);
-}
-
-static void objdb_rdlock(void)
+static void objdb_lock(void)
 {
-	pthread_mutex_lock(&meta_lock);
-	if (lock_thread != pthread_self())
-		pthread_rwlock_rdlock(&reload_lock);
-	pthread_mutex_unlock(&meta_lock);
+	pthread_mutex_lock(&objdb_mutex);
 }
 
-static void objdb_rdunlock(void)
+static void objdb_unlock(void)
 {
-	pthread_mutex_lock(&meta_lock);
-	if (lock_thread != pthread_self())
-		pthread_rwlock_unlock(&reload_lock);
-	pthread_mutex_unlock(&meta_lock);
-}
-
-static void objdb_wrunlock(void)
-{
-	pthread_mutex_lock(&meta_lock);
-	pthread_rwlock_unlock(&reload_lock);
-	lock_thread = 0;
-	pthread_mutex_unlock(&meta_lock);
+	pthread_mutex_unlock(&objdb_mutex);
 }
 
 static int objdb_init (void)
@@ -166,8 +147,11 @@ static int objdb_init (void)
 	list_init (&instance->child_list);
 	list_init (&instance->track_head);
 	list_init (&objdb_trackers_head);
-	pthread_rwlock_init(&reload_lock, NULL);
-	pthread_mutex_init(&meta_lock, NULL);
+
+	pthread_mutexattr_init(&objdb_mutex_attr);
+
+	pthread_mutexattr_settype(&objdb_mutex_attr, PTHREAD_MUTEX_RECURSIVE);
+	pthread_mutex_init(&objdb_mutex, &objdb_mutex_attr);
 
 	hdb_handle_put (&object_instance_database, handle);
 	return (0);
@@ -385,7 +369,7 @@ static int object_create (
 	int found = 0;
 	int i;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_instance_database,
 		parent_object_handle, (void *)&parent_instance);
 	if (res != 0) {
@@ -458,7 +442,7 @@ static int object_create (
 		object_instance->parent_handle,
 		object_instance->object_name,
 		object_instance->object_name_len);
-	objdb_rdunlock();
+	objdb_unlock();
 	return (0);
 
 error_put_destroy:
@@ -471,7 +455,7 @@ error_object_put:
 	hdb_handle_put (&object_instance_database, parent_object_handle);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -482,7 +466,7 @@ static int object_priv_set (
 	int res;
 	struct object_instance *object_instance;
 
-	objdb_rdlock();
+	objdb_lock();
 
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&object_instance);
@@ -493,11 +477,11 @@ static int object_priv_set (
 	object_instance->priv = priv;
 
 	hdb_handle_put (&object_instance_database, object_handle);
-	objdb_rdunlock();
+	objdb_unlock();
 	return (0);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -518,7 +502,7 @@ static int object_key_create_typed(
 	size_t expected_size;
 	int test_size_by_type = CS_TRUE;
 
-	objdb_rdlock();
+	objdb_lock();
 
 	res = hdb_handle_get (&object_instance_database,
 			  object_handle, (void *)&instance);
@@ -638,7 +622,7 @@ static int object_key_create_typed(
 	object_key_changed_notification(object_handle, key_name, key_len,
 		value, value_len, OBJECT_KEY_CREATED);
 	hdb_handle_put (&object_instance_database, object_handle);
-	objdb_rdunlock();
+	objdb_unlock();
 	return (0);
 
 error_put_key:
@@ -651,7 +635,7 @@ error_put:
 	hdb_handle_put (&object_instance_database, object_handle);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -727,12 +711,12 @@ static int object_destroy (
 	struct object_instance *instance;
 	unsigned int res;
 
-	objdb_rdlock();
+	objdb_lock();
 
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&instance);
 	if (res != 0) {
-		objdb_rdunlock();
+		objdb_unlock();
 		return (res);
 	}
 
@@ -749,7 +733,7 @@ static int object_destroy (
 	hdb_handle_put (&object_instance_database, object_handle);
 	hdb_handle_destroy (&object_instance_database, object_handle);
 
-	objdb_rdunlock();
+	objdb_unlock();
 	return (res);
 }
 
@@ -761,7 +745,7 @@ static int object_valid_set (
 	struct object_instance *instance;
 	unsigned int res;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&instance);
 	if (res != 0) {
@@ -773,11 +757,11 @@ static int object_valid_set (
 
 	hdb_handle_put (&object_instance_database, object_handle);
 
-	objdb_rdunlock();
+	objdb_unlock();
 	return (0);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -789,7 +773,7 @@ static int object_key_valid_set (
 	struct object_instance *instance;
 	unsigned int res;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&instance);
 	if (res != 0) {
@@ -801,11 +785,11 @@ static int object_key_valid_set (
 
 	hdb_handle_put (&object_instance_database, object_handle);
 
-	objdb_rdunlock();
+	objdb_unlock();
 	return (0);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -822,7 +806,7 @@ static int object_find_create (
 	struct object_instance *object_instance;
 	struct object_find_instance *object_find_instance;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&object_instance);
 	if (res != 0) {
@@ -848,7 +832,7 @@ static int object_find_create (
 	hdb_handle_put (&object_instance_database, object_handle);
 	hdb_handle_put (&object_find_instance_database, *object_find_handle);
 
-	objdb_rdunlock();
+	objdb_unlock();
 	return (0);
 
 error_destroy:
@@ -858,7 +842,7 @@ error_put:
 	hdb_handle_put (&object_instance_database, object_handle);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -872,7 +856,7 @@ static int object_find_next (
 	struct list_head *list;
 	unsigned int found = 0;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_find_instance_database,
 		object_find_handle, (void *)&object_find_instance);
 	if (res != 0) {
@@ -903,11 +887,11 @@ static int object_find_next (
 		*object_handle = object_instance->object_handle;
 		res = 0;
 	}
-	objdb_rdunlock();
+	objdb_unlock();
 	return (res);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -917,7 +901,7 @@ static int object_find_destroy (
 	struct object_find_instance *object_find_instance;
 	unsigned int res;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_find_instance_database,
 		object_find_handle, (void *)&object_find_instance);
 	if (res != 0) {
@@ -926,11 +910,11 @@ static int object_find_destroy (
 	hdb_handle_put(&object_find_instance_database, object_find_handle);
 	hdb_handle_destroy(&object_find_instance_database, object_find_handle);
 
-	objdb_rdunlock();
+	objdb_unlock();
 	return (0);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -948,7 +932,7 @@ static int object_key_get_typed (
 	int found = 0;
 	size_t key_len = strlen(key_name);
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&instance);
 	if (res != 0) {
@@ -977,11 +961,11 @@ static int object_key_get_typed (
 	}
 
 	hdb_handle_put (&object_instance_database, object_handle);
-	objdb_rdunlock();
+	objdb_unlock();
 	return (res);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -1025,7 +1009,7 @@ static int object_key_increment (
 	struct list_head *list;
 	int found = 0;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&instance);
 	if (res != 0) {
@@ -1091,11 +1075,11 @@ static int object_key_increment (
 		object_key_changed_notification (object_handle, key_name, key_len,
 			object_key->value, object_key->value_len, OBJECT_KEY_REPLACED);
 	}
-	objdb_rdunlock();
+	objdb_unlock();
 	return (res);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -1111,7 +1095,7 @@ static int object_key_decrement (
 	struct list_head *list;
 	int found = 0;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&instance);
 	if (res != 0) {
@@ -1178,11 +1162,11 @@ static int object_key_decrement (
 		object_key_changed_notification (object_handle, key_name, key_len,
 			object_key->value, object_key->value_len, OBJECT_KEY_REPLACED);
 	}
-	objdb_rdunlock();
+	objdb_unlock();
 	return (res);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -1198,7 +1182,7 @@ static int object_key_delete (
 	struct list_head *list;
 	int found = 0;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&instance);
 	if (res != 0) {
@@ -1231,11 +1215,11 @@ static int object_key_delete (
 		object_key_changed_notification(object_handle, key_name, key_len,
 			NULL, 0, OBJECT_KEY_DELETED);
 	}
-	objdb_rdunlock();
+	objdb_unlock();
 	return (ret);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -1254,7 +1238,7 @@ static int object_key_replace (
 	int found = 0;
 	int value_changed = 0;
 
-	objdb_rdlock();
+	objdb_lock();
 
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&instance);
@@ -1338,13 +1322,13 @@ static int object_key_replace (
 		object_key_changed_notification (object_handle, key_name, key_len,
 			new_value, new_value_len, OBJECT_KEY_REPLACED);
 	}
-	objdb_rdunlock();
+	objdb_unlock();
 	return (ret);
 
 error_put:
 	hdb_handle_put (&object_instance_database, object_handle);
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -1355,7 +1339,7 @@ static int object_priv_get (
 	int res;
 	struct object_instance *object_instance;
 
-	objdb_rdunlock();
+	objdb_unlock();
 	res = hdb_handle_get (&object_instance_database,
 		object_handle, (void *)&object_instance);
 	if (res != 0) {
@@ -1365,11 +1349,11 @@ static int object_priv_get (
 	*priv = object_instance->priv;
 
 	hdb_handle_put (&object_instance_database, object_handle);
-	objdb_rdunlock();
+	objdb_unlock();
 	return (0);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -1463,7 +1447,7 @@ static int object_key_iter_reset(hdb_handle_t object_handle)
 	unsigned int res;
 	struct object_instance *instance;
 
-	objdb_rdlock();
+	objdb_lock();
 
 	res = hdb_handle_get (&object_instance_database,
 			      object_handle, (void *)&instance);
@@ -1473,11 +1457,11 @@ static int object_key_iter_reset(hdb_handle_t object_handle)
 	instance->iter_key_list = &instance->key_head;
 
 	hdb_handle_put (&object_instance_database, object_handle);
-	objdb_rdunlock();
+	objdb_unlock();
 	return (0);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -1493,7 +1477,7 @@ static int object_key_iter_typed (hdb_handle_t parent_object_handle,
 	struct list_head *list;
 	unsigned int found = 0;
 
-	objdb_rdlock();
+	objdb_lock();
 
 	res = hdb_handle_get (&object_instance_database,
 		parent_object_handle, (void *)&instance);
@@ -1520,11 +1504,11 @@ static int object_key_iter_typed (hdb_handle_t parent_object_handle,
 	}
 
 	hdb_handle_put (&object_instance_database, parent_object_handle);
-	objdb_rdunlock();
+	objdb_unlock();
 	return (res);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -1560,7 +1544,7 @@ static int object_key_iter_from(hdb_handle_t parent_object_handle,
 	struct list_head *list;
 	unsigned int found = 0;
 
-	objdb_rdlock();
+	objdb_lock();
 
 	res = hdb_handle_get (&object_instance_database,
 		parent_object_handle, (void *)&instance);
@@ -1594,11 +1578,11 @@ static int object_key_iter_from(hdb_handle_t parent_object_handle,
 	}
 
 	hdb_handle_put (&object_instance_database, parent_object_handle);
-	objdb_rdunlock();
+	objdb_unlock();
 	return (res);
 
 error_exit:
-	objdb_rdunlock();
+	objdb_unlock();
 	return (-1);
 }
 
@@ -1609,12 +1593,12 @@ static int object_parent_get(hdb_handle_t object_handle,
 	struct object_instance *instance;
 	unsigned int res;
 
-	objdb_rdlock();
+	objdb_lock();
 
 	res = hdb_handle_get (&object_instance_database,
 			      object_handle, (void *)&instance);
 	if (res != 0) {
-		objdb_rdunlock();
+		objdb_unlock();
 		return (res);
 	}
 
@@ -1624,7 +1608,7 @@ static int object_parent_get(hdb_handle_t object_handle,
 		*parent_handle = instance->parent_handle;
 
 	hdb_handle_put (&object_instance_database, object_handle);
-	objdb_rdunlock();
+	objdb_unlock();
 
 	return (0);
 }
@@ -1636,11 +1620,11 @@ static int object_name_get(hdb_handle_t object_handle,
 	struct object_instance *instance;
 	unsigned int res;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_instance_database,
 			      object_handle, (void *)&instance);
 	if (res != 0) {
-		objdb_rdunlock();
+		objdb_unlock();
 		return (res);
 	}
 
@@ -1648,7 +1632,7 @@ static int object_name_get(hdb_handle_t object_handle,
 	*object_name_len = instance->object_name_len;
 
 	hdb_handle_put (&object_instance_database, object_handle);
-	objdb_rdunlock();
+	objdb_unlock();
 
 	return (0);
 }
@@ -1747,11 +1731,11 @@ static int object_dump(hdb_handle_t object_handle,
 	struct object_instance *instance;
 	unsigned int res;
 
-	objdb_rdlock();
+	objdb_lock();
 	res = hdb_handle_get (&object_instance_database,
 			      object_handle, (void *)&instance);
 	if (res != 0) {
-		objdb_rdunlock();
+		objdb_unlock();
 		return (res);
 	}
 
@@ -1759,7 +1743,7 @@ static int object_dump(hdb_handle_t object_handle,
 
 	hdb_handle_put (&object_instance_database, object_handle);
 
-	objdb_rdunlock();
+	objdb_unlock();
 	return (res);
 }
 
@@ -1772,18 +1756,18 @@ static int object_write_config(const char **error_string)
 
 	main_get_config_modules(&modules, &num_modules);
 
-	objdb_wrlock();
+	objdb_lock();
 
 	for (i=0; i<num_modules; i++) {
 		if (modules[i]->config_writeconfig) {
 			res = modules[i]->config_writeconfig(&objdb_iface, error_string);
 			if (res) {
-				objdb_wrunlock();
+				objdb_unlock();
 				return res;
 			}
 		}
 	}
-	objdb_wrunlock();
+	objdb_unlock();
 	return 0;
 }
 
@@ -1797,19 +1781,19 @@ static int object_reload_config(int flush, const char **error_string)
 	main_get_config_modules(&modules, &num_modules);
 	object_reload_notification(OBJDB_RELOAD_NOTIFY_START, flush);
 
-	objdb_wrlock();
+	objdb_lock();
 
 	for (i=0; i<num_modules; i++) {
 		if (modules[i]->config_reloadconfig) {
 			res = modules[i]->config_reloadconfig(&objdb_iface, flush, error_string);
 			if (res) {
 				object_reload_notification(OBJDB_RELOAD_NOTIFY_FAILED, flush);
-				objdb_wrunlock();
+				objdb_unlock();
 				return res;
 			}
 		}
 	}
-	objdb_wrunlock();
+	objdb_unlock();
 	object_reload_notification(OBJDB_RELOAD_NOTIFY_END, flush);
 	return 0;
 }