Hi,
After some investigation, I found out that the problem is that different
SPICE threads are calling monitor functions (such as
monitor_protocol_event()) in parallel which causes concurrent access
to the monitor's internal buffer outbuf[].
[ adding spice-list to Cc, see qemu-devel for the rest of the thread ]
spice isn't supposed to do that.
/me just added a assert in channel_event() and saw it trigger in display
channel disconnects.
#0 0x0000003ceba32a45 in raise () from /lib64/libc.so.6
#1 0x0000003ceba34225 in abort () from /lib64/libc.so.6
#2 0x0000003ceba2b9d5 in __assert_fail () from /lib64/libc.so.6
#3 0x0000000000503759 in channel_event (event=3, info=0x35e9340)
at /home/kraxel/projects/qemu/ui/spice-core.c:223
#4 0x00007f9a77a9921b in reds_channel_event (s=0x35e92c0) at reds.c:400
#5 reds_stream_free (s=0x35e92c0) at reds.c:4981
#6 0x00007f9a77aac8b0 in red_disconnect_channel
(channel=0x7f9a24069a80) at red_worker.c:8489
#7 0x00007f9a77ab53a8 in handle_dev_input (listener=0x7f9a3211ab20,
events=<value optimized out>)
at red_worker.c:10062
#8 0x00007f9a77ab436d in red_worker_main (arg=<value optimized out>) at
red_worker.c:10304
#9 0x0000003cec2077e1 in start_thread () from /lib64/libpthread.so.0
#10 0x0000003cebae68ed in clone () from /lib64/libc.so.6
IMHO spice server should handle the display channel tear-down in the
dispatcher instead of the worker thread. Alon?
Anyways, this commit fixes the problem at hand.
Not really. channel_event() itself isn't thread-safe too, it does
unlocked list operations which can also blow up when called from
different threads.
A patch like the attached (warning: untested) should do as quick&dirty
fix for stable. But IMO we really should fix spice instead.
cheers,
Gerd
>From 7496e573ff6085d3c42d7e65b72c85fd2a7b4a78 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kra...@redhat.com>
Date: Fri, 2 Sep 2011 15:03:28 +0200
Subject: [PATCH] spice: workaround a spice server bug.
---
ui/spice-core.c | 21 ++++++++++++++++++++-
1 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index dba11f0..c99cdc5 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -19,6 +19,7 @@
#include <spice-experimental.h>
#include <netdb.h>
+#include <pthread.h>
#include "qemu-common.h"
#include "qemu-spice.h"
@@ -44,6 +45,8 @@ static char *auth_passwd;
static time_t auth_expires = TIME_MAX;
int using_spice = 0;
+static pthread_t me;
+
struct SpiceTimer {
QEMUTimer *timer;
QTAILQ_ENTRY(SpiceTimer) next;
@@ -216,6 +219,8 @@ static void channel_event(int event, SpiceChannelEventInfo
*info)
};
QDict *server, *client;
QObject *data;
+ bool need_lock = !pthread_equal(me, pthread_self());
+ static int first = 1;
client = qdict_new();
add_addr_info(client, &info->paddr, info->plen);
@@ -223,6 +228,14 @@ static void channel_event(int event, SpiceChannelEventInfo
*info)
server = qdict_new();
add_addr_info(server, &info->laddr, info->llen);
+ if (need_lock) {
+ qemu_mutex_lock_iothread();
+ if (first) {
+ fprintf(stderr, "You are using a broken spice-server version\n");
+ first = 0;
+ }
+ }
+
if (event == SPICE_CHANNEL_EVENT_INITIALIZED) {
qdict_put(server, "auth", qstring_from_str(auth));
add_channel_info(client, info);
@@ -236,6 +249,10 @@ static void channel_event(int event, SpiceChannelEventInfo
*info)
QOBJECT(client), QOBJECT(server));
monitor_protocol_event(qevent[event], data);
qobject_decref(data);
+
+ if (need_lock) {
+ qemu_mutex_unlock_iothread();
+ }
}
#else /* SPICE_INTERFACE_CORE_MINOR >= 3 */
@@ -482,7 +499,9 @@ void qemu_spice_init(void)
spice_image_compression_t compression;
spice_wan_compression_t wan_compr;
- if (!opts) {
+ me = pthread_self();
+
+ if (!opts) {
return;
}
port = qemu_opt_get_number(opts, "port", 0);
--
1.7.1
