[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [RFC v2 01/22] char-io: fix possible race on IOWatchPoll
From: |
Peter Xu |
Subject: |
[Qemu-devel] [RFC v2 01/22] char-io: fix possible race on IOWatchPoll |
Date: |
Fri, 29 Sep 2017 11:38:23 +0800 |
This is not a problem if we are only having one single loop thread like
before. However, after per-monitor thread is introduced, this is not
true any more, and the race can happen.
The race can be triggered with "make check -j8" sometimes:
qemu-system-x86_64: /root/git/qemu/chardev/char-io.c:91:
io_watch_poll_finalize: Assertion `iwp->src == NULL' failed.
This patch keeps the reference for the watch object when creating in
io_add_watch_poll(), so that the object will never be released in the
context main loop, especially when the context loop is running in
another standalone thread. Meanwhile, when we want to remove the watch
object, we always first detach the watch object from its owner context,
then we continue with the cleanup.
Without this patch, calling io_remove_watch_poll() in main loop thread
is not thread-safe, since the other per-monitor thread may be modifying
the watch object at the same time.
Reviewed-by: Marc-André Lureau <address@hidden>
Signed-off-by: Peter Xu <address@hidden>
---
chardev/char-io.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/chardev/char-io.c b/chardev/char-io.c
index f81052481a..50b5bac704 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -122,7 +122,6 @@ GSource *io_add_watch_poll(Chardev *chr,
g_free(name);
g_source_attach(&iwp->parent, context);
- g_source_unref(&iwp->parent);
return (GSource *)iwp;
}
@@ -131,12 +130,25 @@ static void io_remove_watch_poll(GSource *source)
IOWatchPoll *iwp;
iwp = io_watch_poll_from_source(source);
+
+ /*
+ * Here the order of destruction really matters. We need to first
+ * detach the IOWatchPoll object from the context (which may still
+ * be running in another loop thread), only after that could we
+ * continue to operate on iwp->src, or there may be race condition
+ * between current thread and the context loop thread.
+ *
+ * Let's blame the glib bug mentioned in commit 2b316774f6
+ * ("qemu-char: do not operate on sources from finalize
+ * callbacks") for this extra complexity.
+ */
+ g_source_destroy(&iwp->parent);
if (iwp->src) {
g_source_destroy(iwp->src);
g_source_unref(iwp->src);
iwp->src = NULL;
}
- g_source_destroy(&iwp->parent);
+ g_source_unref(&iwp->parent);
}
void remove_fd_in_watch(Chardev *chr)
--
2.13.5
- [Qemu-devel] [RFC v2 00/22] QMP: out-of-band (OOB) execution support, Peter Xu, 2017/09/28
- [Qemu-devel] [RFC v2 01/22] char-io: fix possible race on IOWatchPoll,
Peter Xu <=
- [Qemu-devel] [RFC v2 02/22] qobject: introduce qstring_get_try_str(), Peter Xu, 2017/09/28
- [Qemu-devel] [RFC v2 03/22] qobject: introduce qobject_get_try_str(), Peter Xu, 2017/09/28
- [Qemu-devel] [RFC v2 04/22] qobject: let object_property_get_str() use new API, Peter Xu, 2017/09/28
- [Qemu-devel] [RFC v2 05/22] monitor: move skip_flush into monitor_data_init, Peter Xu, 2017/09/28
- [Qemu-devel] [RFC v2 06/22] qjson: add "opaque" field to JSONMessageParser, Peter Xu, 2017/09/28
- [Qemu-devel] [RFC v2 08/22] monitor: unify global init, Peter Xu, 2017/09/28
- [Qemu-devel] [RFC v2 07/22] monitor: move the cur_mon hack deeper for QMP, Peter Xu, 2017/09/28
- [Qemu-devel] [RFC v2 09/22] monitor: create monitor dedicate iothread, Peter Xu, 2017/09/28
- [Qemu-devel] [RFC v2 10/22] monitor: allow to use IO thread for parsing, Peter Xu, 2017/09/28
- [Qemu-devel] [RFC v2 11/22] monitor: introduce monitor_qmp_respond(), Peter Xu, 2017/09/28