aboutsummaryrefslogtreecommitdiff
path: root/ui
diff options
context:
space:
mode:
authorCorentin Chary <corentin.chary@gmail.com>2012-03-14 07:58:47 +0100
committerAnthony Liguori <aliguori@us.ibm.com>2012-03-14 16:22:46 -0500
commit175b2a6e4be06422da59d3a82c28d9a0e738e282 (patch)
tree109f343f1e5207c3c6a2fc4c339465f4a1e81dc6 /ui
parent418ba9e5d6849ef2e8512d8853628ce4bf37937a (diff)
downloadqemu-175b2a6e4be06422da59d3a82c28d9a0e738e282.zip
qemu-175b2a6e4be06422da59d3a82c28d9a0e738e282.tar.gz
qemu-175b2a6e4be06422da59d3a82c28d9a0e738e282.tar.bz2
vnc: don't mess up with iohandlers in the vnc thread
The threaded VNC servers messed up with QEMU fd handlers without any kind of locking, and that can cause some nasty race conditions. Using qemu_mutex_lock_iothread() won't work because vnc_dpy_cpy(), which will wait for the current job queue to finish, can be called with the iothread lock held. Instead, we now store the data in a temporary buffer, and use a bottom half to notify the main thread that new data is available. vnc_[un]lock_ouput() is still needed to access VncState members like abort, csock or jobs_buffer. Signed-off-by: Corentin Chary <corentin.chary@gmail.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Diffstat (limited to 'ui')
-rw-r--r--ui/vnc-jobs-async.c48
-rw-r--r--ui/vnc-jobs.h1
-rw-r--r--ui/vnc.c12
-rw-r--r--ui/vnc.h2
4 files changed, 44 insertions, 19 deletions
diff --git a/ui/vnc-jobs-async.c b/ui/vnc-jobs-async.c
index 9b3016c..087b84d 100644
--- a/ui/vnc-jobs-async.c
+++ b/ui/vnc-jobs-async.c
@@ -28,6 +28,7 @@
#include "vnc.h"
#include "vnc-jobs.h"
+#include "qemu_socket.h"
/*
* Locking:
@@ -155,6 +156,24 @@ void vnc_jobs_join(VncState *vs)
qemu_cond_wait(&queue->cond, &queue->mutex);
}
vnc_unlock_queue(queue);
+ vnc_jobs_consume_buffer(vs);
+}
+
+void vnc_jobs_consume_buffer(VncState *vs)
+{
+ bool flush;
+
+ vnc_lock_output(vs);
+ if (vs->jobs_buffer.offset) {
+ vnc_write(vs, vs->jobs_buffer.buffer, vs->jobs_buffer.offset);
+ buffer_reset(&vs->jobs_buffer);
+ }
+ flush = vs->csock != -1 && vs->abort != true;
+ vnc_unlock_output(vs);
+
+ if (flush) {
+ vnc_flush(vs);
+ }
}
/*
@@ -197,7 +216,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
VncState vs;
int n_rectangles;
int saved_offset;
- bool flush;
vnc_lock_queue(queue);
while (QTAILQ_EMPTY(&queue->jobs) && !queue->exit) {
@@ -213,6 +231,7 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
vnc_lock_output(job->vs);
if (job->vs->csock == -1 || job->vs->abort == true) {
+ vnc_unlock_output(job->vs);
goto disconnected;
}
vnc_unlock_output(job->vs);
@@ -233,10 +252,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
if (job->vs->csock == -1) {
vnc_unlock_display(job->vs->vd);
- /* output mutex must be locked before going to
- * disconnected:
- */
- vnc_lock_output(job->vs);
goto disconnected;
}
@@ -254,24 +269,19 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
vs.output.buffer[saved_offset] = (n_rectangles >> 8) & 0xFF;
vs.output.buffer[saved_offset + 1] = n_rectangles & 0xFF;
- /* Switch back buffers */
vnc_lock_output(job->vs);
- if (job->vs->csock == -1) {
- goto disconnected;
+ if (job->vs->csock != -1) {
+ buffer_reserve(&job->vs->jobs_buffer, vs.output.offset);
+ buffer_append(&job->vs->jobs_buffer, vs.output.buffer,
+ vs.output.offset);
+ /* Copy persistent encoding data */
+ vnc_async_encoding_end(job->vs, &vs);
+
+ qemu_bh_schedule(job->vs->bh);
}
-
- vnc_write(job->vs, vs.output.buffer, vs.output.offset);
-
-disconnected:
- /* Copy persistent encoding data */
- vnc_async_encoding_end(job->vs, &vs);
- flush = (job->vs->csock != -1 && job->vs->abort != true);
vnc_unlock_output(job->vs);
- if (flush) {
- vnc_flush(job->vs);
- }
-
+disconnected:
vnc_lock_queue(queue);
QTAILQ_REMOVE(&queue->jobs, job, next);
vnc_unlock_queue(queue);
diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h
index b8dab81..4c661f9 100644
--- a/ui/vnc-jobs.h
+++ b/ui/vnc-jobs.h
@@ -40,6 +40,7 @@ void vnc_jobs_join(VncState *vs);
#ifdef CONFIG_VNC_THREAD
+void vnc_jobs_consume_buffer(VncState *vs);
void vnc_start_worker_thread(void);
bool vnc_worker_thread_running(void);
void vnc_stop_worker_thread(void);
diff --git a/ui/vnc.c b/ui/vnc.c
index bdec33a..aef6d3a 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1068,7 +1068,10 @@ static void vnc_disconnect_finish(VncState *vs)
#ifdef CONFIG_VNC_THREAD
qemu_mutex_destroy(&vs->output_mutex);
+ qemu_bh_delete(vs->bh);
+ buffer_free(&vs->jobs_buffer);
#endif
+
for (i = 0; i < VNC_STAT_ROWS; ++i) {
g_free(vs->lossy_rect[i]);
}
@@ -1283,6 +1286,14 @@ static long vnc_client_read_plain(VncState *vs)
return ret;
}
+#ifdef CONFIG_VNC_THREAD
+static void vnc_jobs_bh(void *opaque)
+{
+ VncState *vs = opaque;
+
+ vnc_jobs_consume_buffer(vs);
+}
+#endif
/*
* First function called whenever there is more data to be read from
@@ -2687,6 +2698,7 @@ static void vnc_connect(VncDisplay *vd, int csock, int skipauth)
#ifdef CONFIG_VNC_THREAD
qemu_mutex_init(&vs->output_mutex);
+ vs->bh = qemu_bh_new(vnc_jobs_bh, vs);
#endif
QTAILQ_INSERT_HEAD(&vd->clients, vs, next);
diff --git a/ui/vnc.h b/ui/vnc.h
index 0bd1fc6..a851ebd 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -304,6 +304,8 @@ struct VncState
VncJob job;
#else
QemuMutex output_mutex;
+ QEMUBH *bh;
+ Buffer jobs_buffer;
#endif
/* Encoding specific, if you add something here, don't forget to