All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Gerd Hoffmann <kraxel@redhat.com>, qemu-devel@nongnu.org
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: Re: [PULL 09/11] ui/vnc: clipboard support
Date: Mon, 20 Dec 2021 17:33:37 +0300	[thread overview]
Message-ID: <973ddebe-14a9-4ba7-c389-7a97d6017237@virtuozzo.com> (raw)
In-Reply-To: <20210521125119.3173309-10-kraxel@redhat.com>

21.05.2021 15:51, Gerd Hoffmann wrote:
> This patch adds support for cut+paste to the qemu vnc server, which
> allows the vnc client exchange clipbaord data with qemu and other peers
> like the qemu vdagent implementation.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-id: 20210519053940.1888907-1-kraxel@redhat.com
> Message-Id: <20210519053940.1888907-8-kraxel@redhat.com>
> ---

[..]

> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -25,6 +25,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu-common.h"
>   #include "vnc.h"
>   #include "vnc-jobs.h"
>   #include "trace.h"
> @@ -1352,6 +1353,9 @@ void vnc_disconnect_finish(VncState *vs)
>           /* last client gone */
>           vnc_update_server_surface(vs->vd);
>       }
> +    if (vs->cbpeer.update.notify) {
> +        qemu_clipboard_peer_unregister(&vs->cbpeer);
> +    }
>   
>       vnc_unlock_output(vs);
>   

Hi Gerd!

Something is wrong here.

We are already under mutex, but calling qemu_clipboard_peer_unregister may trigger taking this mutex again in same call stack:

(gdb) bt
#0  0x00007f4c979b77b0 in __lll_lock_wait () at /lib64/libpthread.so.0
#1  0x00007f4c979b0553 in pthread_mutex_lock () at /lib64/libpthread.so.0
#2  0x0000558fb33bbb72 in qemu_mutex_lock_impl (mutex=0x558fb5416598, file=0x558fb3447fb7 "../ui/vnc-jobs.h", line=60) at ../util/qemu-thread-posix.c:80
#3  0x0000558fb2ca16c6 in vnc_lock_output (vs=0x558fb540a400) at ../ui/vnc-jobs.h:60
#4  0x0000558fb2ca1ab0 in vnc_clipboard_send (vs=0x558fb540a400, count=1, dwords=0x7ffe37f5f968) at ../ui/vnc-clipboard.c:138
#5  0x0000558fb2ca1f17 in vnc_clipboard_notify (notifier=0x558fb541a700, data=0x558fb524ee70) at ../ui/vnc-clipboard.c:209
#6  0x0000558fb33c25f0 in notifier_list_notify (list=0x558fb3a142f0 <clipboard_notifiers>, data=0x558fb524ee70) at ../util/notify.c:39
#7  0x0000558fb2c6d601 in qemu_clipboard_update (info=0x558fb524ee70) at ../ui/clipboard.c:50
#8  0x0000558fb2c6d570 in qemu_clipboard_peer_release (peer=0x558fb541a6f8, selection=QEMU_CLIPBOARD_SELECTION_CLIPBOARD) at ../ui/clipboard.c:41
#9  0x0000558fb2c6d4b6 in qemu_clipboard_peer_unregister (peer=0x558fb541a6f8) at ../ui/clipboard.c:19
#10 0x0000558fb2c8518a in vnc_disconnect_finish (vs=0x558fb540a400) at ../ui/vnc.c:1358
#11 0x0000558fb2c848b0 in vnc_update_client (vs=0x558fb540a400, has_dirty=0) at ../ui/vnc.c:1167
#12 0x0000558fb2c8a1ce in vnc_refresh (dcl=0x558fb5882610) at ../ui/vnc.c:3207
#13 0x0000558fb2c72dec in dpy_refresh (s=0x558fb4fe7970) at ../ui/console.c:1673
#14 0x0000558fb2c6ebe4 in gui_update (opaque=0x558fb4fe7970) at ../ui/console.c:158
#15 0x0000558fb33e862b in timerlist_run_timers (timer_list=0x558fb45ed200) at ../util/qemu-timer.c:573
#16 0x0000558fb33e86d5 in qemu_clock_run_timers (type=QEMU_CLOCK_REALTIME) at ../util/qemu-timer.c:587
#17 0x0000558fb33e899a in qemu_clock_run_all_timers () at ../util/qemu-timer.c:669
#18 0x0000558fb33e26ec in main_loop_wait (nonblocking=0) at ../util/main-loop.c:542
#19 0x0000558fb3099c75 in qemu_main_loop () at ../softmmu/runstate.c:726
#20 0x0000558fb2c62ae6 in main (argc=5, argv=0x7ffe37f5fdf8, envp=0x7ffe37f5fe28) at ../softmmu/main.c:50
(gdb) fr 2
#2  0x0000558fb33bbb72 in qemu_mutex_lock_impl (mutex=0x558fb5416598, file=0x558fb3447fb7 "../ui/vnc-jobs.h", line=60) at ../util/qemu-thread-posix.c:80
80          err = pthread_mutex_lock(&mutex->lock);
(gdb) p *mutex
$1 = {lock = {__data = {__lock = 2, __count = 0, __owner = 1549902, __nusers = 1, __kind = 0, __spins = 0, __elision = 0, __list = {__prev = 0x0, __next = 0x0}},
     __size = "\002\000\000\000\000\000\000\000N\246\027\000\001", '\000' <repeats 26 times>, __align = 2}, file = 0x558fb3442aaf "../ui/vnc-jobs.h", line = 60, initialized = true}
(gdb) info thr
   Id   Target Id                                             Frame
* 1    Thread 0x7f4c89ba8fc0 (LWP 1549902) "qemu-system-x86" 0x00007f4c979b77b0 in __lll_lock_wait () from /lib64/libpthread.so.0
   2    Thread 0x7f4c89a36640 (LWP 1549903) "qemu-system-x86" 0x00007f4c96ad8e0d in syscall () from /lib64/libc.so.6
   3    Thread 0x7f4c890b3640 (LWP 1549904) "qemu-system-x86" 0x00007f4c96ad35bf in poll () from /lib64/libc.so.6
   4    Thread 0x7f4c03fff640 (LWP 1549905) "qemu-system-x86" 0x00007f4c979baa8a in __futex_abstimed_wait_common64 () from /lib64/libpthread.so.0
   5    Thread 0x7f4c01bff640 (LWP 1549906) "qemu-system-x86" 0x00007f4c979baa8a in __futex_abstimed_wait_common64 () from /lib64/libpthread.so.0


Reproduce is simple:

On master branch, run something like this:

   ./build/qemu-system-x86_64 -vnc :7 --qmp stdio

Then, in two different terminals start connecting loops:

   while true; do vncviewer <vnc server ip>:5907; done

Then wait. If vncviewer loop stack for some reason, when qemu is not yet dead-locked, just restart the loop. Qemu screen becoming black - good sign of dead-lock happen, go into gdb and check.
By default vncviewer does non-shared connection, so previous client is closed when new one is connected. So we trigger many disconnects and connects in a loop.


I don't know this code, so I'm not sure that it would be safe just move qemu_clipboard_peer_unregister() out of the critical section...

-- 
Best regards,
Vladimir


  reply	other threads:[~2021-12-20 15:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-21 12:51 [PULL 00/11] Ui 20210521 patches Gerd Hoffmann
2021-05-21 12:51 ` [PULL 01/11] vnc: spelling fix (enable->enabled) Gerd Hoffmann
2021-05-21 12:51 ` [PULL 02/11] ui/spice-display: check NULL pointer in interface_release_resource() Gerd Hoffmann
2021-05-21 12:51 ` [PULL 03/11] build: add separate spice-protocol config option Gerd Hoffmann
2021-05-21 12:51 ` [PULL 04/11] ui: add clipboard infrastructure Gerd Hoffmann
2021-05-21 12:51 ` [PULL 05/11] ui: add clipboard documentation Gerd Hoffmann
2021-05-21 12:51 ` [PULL 06/11] ui/vdagent: core infrastructure Gerd Hoffmann
2021-05-21 12:51 ` [PULL 07/11] ui/vdagent: add mouse support Gerd Hoffmann
2021-05-21 12:51 ` [PULL 08/11] ui/vdagent: add clipboard support Gerd Hoffmann
2021-05-21 12:51 ` [PULL 09/11] ui/vnc: " Gerd Hoffmann
2021-12-20 14:33   ` Vladimir Sementsov-Ogievskiy [this message]
2021-05-21 12:51 ` [PULL 10/11] ui/gtk: move struct GtkDisplayState to ui/gtk.h Gerd Hoffmann
2021-05-21 12:51 ` [PULL 11/11] ui/gtk: add clipboard support Gerd Hoffmann
2021-05-21 13:09 ` [PULL 00/11] Ui 20210521 patches no-reply
2021-05-21 15:23 ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=973ddebe-14a9-4ba7-c389-7a97d6017237@virtuozzo.com \
    --to=vsementsov@virtuozzo.com \
    --cc=armbru@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --subject='Re: [PULL 09/11] ui/vnc: clipboard support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.