spice-display: fix qemu_spice_cursor_refresh_bh locking

spice-display should not call the ui/console.c functions dpy_cursor_define
and dpy_moues_set with the SimpleSpiceDisplay lock taken.  That will cause
a deadlock, because the DisplayChangeListener callbacks will take the lock
again.  It is also in general a bad idea to invoke generic callbacks with a
lock taken, because it can cause AB-BA deadlocks in the long run.  The only
thing that requires care is that the cursor may disappear as soon as the
mutex is released, so you need an extra cursor_get/cursor_put pair.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 20180720063109.4631-3-pbonzini@redhat.com

[ kraxel: fix dpy_cursor_define() call ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

diff --git a/ui/spice-display.c b/ui/spice-display.c
index 46df673cd75744468fcf754440e1040180bd9df4..7b230987dcb7bf2da86fa0695c2ec10e348dc425 100644 (file)
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -450,29 +450,35 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
     qemu_mutex_unlock(&ssd->lock);
 }
 
-static void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
+void qemu_spice_cursor_refresh_bh(void *opaque)
 {
+    SimpleSpiceDisplay *ssd = opaque;
+
+    qemu_mutex_lock(&ssd->lock);
     if (ssd->cursor) {
+        QEMUCursor *c = ssd->cursor;
         assert(ssd->dcl.con);
-        dpy_cursor_define(ssd->dcl.con, ssd->cursor);
+        cursor_get(c);
+        qemu_mutex_unlock(&ssd->lock);
+        dpy_cursor_define(ssd->dcl.con, c);
+        qemu_mutex_lock(&ssd->lock);
+        cursor_put(c);
     }
+
     if (ssd->mouse_x != -1 && ssd->mouse_y != -1) {
+        int x, y;
         assert(ssd->dcl.con);
-        dpy_mouse_set(ssd->dcl.con, ssd->mouse_x, ssd->mouse_y, 1);
+        x = ssd->mouse_x;
+        y = ssd->mouse_y;
         ssd->mouse_x = -1;
         ssd->mouse_y = -1;
+        qemu_mutex_unlock(&ssd->lock);
+        dpy_mouse_set(ssd->dcl.con, x, y, 1);
+    } else {
+        qemu_mutex_unlock(&ssd->lock);
     }
 }
 
-void qemu_spice_cursor_refresh_bh(void *opaque)
-{
-    SimpleSpiceDisplay *ssd = opaque;
-
-    qemu_mutex_lock(&ssd->lock);
-    qemu_spice_cursor_refresh_unlocked(ssd);
-    qemu_mutex_unlock(&ssd->lock);
-}
-
 void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
 {
     graphic_hw_update(ssd->dcl.con);