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 mutexes. Also every function is locked.
Regards,
Honza
diff --git a/trunk/exec/objdb.c b/trunk/exec/objdb.c
index c15ca86..92b0205 100644
--- a/trunk/exec/objdb.c
+++ b/trunk/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;
}
_______________________________________________
Openais mailing list
Openais@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/openais
