Kevin Greßlehner (1):
Fix #2553: Fix for Deadlock by aligning the lockorder - Lock
&memdb->mutex in memdb_read and refer to a new method
"memdb_read_nolock" in memdb.c which doesn't handle locks by itself.
This method then handles the stuff which was originally in
memdb_read. Therefore everything except
cfs_create_guest_conf_property_msg uses memdb_read (which handles
the locking itself), and cfs_create_guest_conf_property_msg prelocks
&memdb->mutex and invokes memdb_read_nolock.
Signed-off-by: Kevin Greßlehner <kevin_gre...@live.at>
---
Approach #3 (Comment #7 of Bug #2553):
Tested for 6h straight - no Crashes nor Deadlocks appeared.
data/src/memdb.c | 42 +++++++++++++++++++++++++++---------------
data/src/memdb.h | 6 ++++++
data/src/status.c | 12 ++++++++----
3 files changed, 41 insertions(+), 19 deletions(-)
diff --git a/data/src/memdb.c b/data/src/memdb.c
index efc99ea..6c268df 100644
--- a/data/src/memdb.c
+++ b/data/src/memdb.c
@@ -659,32 +659,44 @@ int memdb_mkdir(
return ret;
}
+//Original memdb_read without locking - Caller MUST handle the locking - Fix
for #2553
+int
+memdb_read_nolock(
+ memdb_t *memdb,
+ const char *path,
+ gpointer *data_ret)
+{
+ g_return_val_if_fail(memdb != NULL, -EINVAL);
+ g_return_val_if_fail(path != NULL, -EINVAL);
+ g_return_val_if_fail(data_ret != NULL, -EINVAL);
+
+ memdb_tree_entry_t *te, *parent;
+
+ if ((te = memdb_lookup_path(memdb, path, &parent))) {
+ if (te->type == DT_REG) {
+ *data_ret = g_memdup(te->data.value, te->size);
+ guint32 size = te->size;
+ return size;
+ }
+ }
+
+ return -ENOENT;
+}
+
int
memdb_read(
memdb_t *memdb,
const char *path,
gpointer *data_ret)
{
- g_return_val_if_fail(memdb != NULL, -EINVAL);
- g_return_val_if_fail(path != NULL, -EINVAL);
- g_return_val_if_fail(data_ret != NULL, -EINVAL);
-
- memdb_tree_entry_t *te, *parent;
-
+ int res;
g_mutex_lock (&memdb->mutex);
- if ((te = memdb_lookup_path(memdb, path, &parent))) {
- if (te->type == DT_REG) {
- *data_ret = g_memdup(te->data.value, te->size);
- guint32 size = te->size;
- g_mutex_unlock (&memdb->mutex);
- return size;
- }
- }
+ res = memdb_read_nolock(memdb, path, data_ret);
g_mutex_unlock (&memdb->mutex);
- return -ENOENT;
+ return res;
}
static int
diff --git a/data/src/memdb.h b/data/src/memdb.h
index 60df035..7659358 100644
--- a/data/src/memdb.h
+++ b/data/src/memdb.h
@@ -162,6 +162,12 @@ memdb_read(
const char *path,
gpointer *data_ret);
+int
+memdb_read_nolock(
+ memdb_t *memdb,
+ const char *path,
+ gpointer *data_ret);
+
int
memdb_create(
memdb_t *memdb,
diff --git a/data/src/status.c b/data/src/status.c
index 491082c..1dd2737 100644
--- a/data/src/status.c
+++ b/data/src/status.c
@@ -883,6 +883,9 @@ cfs_create_guest_conf_property_msg(GString *str, memdb_t
*memdb, const char *pro
int res = 0;
GString *path = NULL;
+ //Prelock &memdb->mutex in order to enable the usage of
memdb_read_nolock
+ //to prevent Deadlocks as in #2553
+ g_mutex_lock (&memdb->mutex);
g_mutex_lock (&mutex);
g_string_printf(str,"{\n");
@@ -899,8 +902,8 @@ cfs_create_guest_conf_property_msg(GString *str, memdb_t
*memdb, const char *pro
if (vminfo == NULL) goto enoent;
if (!vminfo_to_path(vminfo, path)) goto err;
-
- int size = memdb_read(memdb, path->str, &tmp);
+ //use memdb_read_nolock because lock is handled here
+ int size = memdb_read_nolock(memdb, path->str, &tmp);
if (tmp == NULL) goto err;
if (size <= prop_len) goto ret;
@@ -924,7 +927,8 @@ cfs_create_guest_conf_property_msg(GString *str, memdb_t
*memdb, const char *pro
g_free(tmp); // no-op if already null
tmp = NULL;
- int size = memdb_read(memdb, path->str, &tmp);
+ //use memdb_read_nolock because lock is handled here
+ int size = memdb_read_nolock(memdb, path->str, &tmp);
if (tmp == NULL || size <= prop_len) continue;
char *val = _get_property_value(tmp, size, prop,
prop_len);
@@ -945,7 +949,7 @@ ret:
}
g_string_append_printf(str,"\n}\n");
g_mutex_unlock (&mutex);
-
+ g_mutex_unlock (&memdb->mutex);
return res;
err:
res = -EIO;
--
2.20.1
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
