ui/spice: Fix abort on SPICE client connect

[PATCH] ui/spice: Fix abort on SPICE client connect

Posted by Mohamed Akram 1 week ago

The BQL is already locked by the main loop, so avoid locking again
unless needed to not trigger an assertion failure.

Signed-off-by: Mohamed Akram <mohd.akram@outlook.com>
---
 ui/spice-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 5992f9d..31c31f2 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -222,7 +222,7 @@ static void channel_event(int event, SpiceChannelEventInfo *info)
      * thread and grab the BQL if so before calling qemu
      * functions.
      */
-    bool need_lock = !qemu_thread_is_self(&me);
+    bool need_lock = !bql_locked();
     if (need_lock) {
         bql_lock();
     }
-- 
2.51.0

Re: [PATCH] ui/spice: Fix abort on SPICE client connect

Posted by Michael Tokarev 1 day, 6 hours ago

On 20.09.2025 23:55, Mohamed Akram wrote:
> The BQL is already locked by the main loop, so avoid locking again
> unless needed to not trigger an assertion failure.
> 
> Signed-off-by: Mohamed Akram <mohd.akram@outlook.com>

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3070
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

And applied to the trivial-pathes tree.  Thank you!

/mjt

Re: [PATCH] ui/spice: Fix abort on SPICE client connect

Posted by Michael Tokarev 1 day, 5 hours ago

On 27.09.2025 11:44, Michael Tokarev wrote:
> On 20.09.2025 23:55, Mohamed Akram wrote:
>> The BQL is already locked by the main loop, so avoid locking again
>> unless needed to not trigger an assertion failure.
>>
>> Signed-off-by: Mohamed Akram <mohd.akram@outlook.com>
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3070
> Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
> 
> And applied to the trivial-pathes tree.  Thank you!

Um.  This is definitely NOT a trivial change, it shouldn't
go through trivial-patches.  While the change itself is a
one-liner, but the logic behind this is.. non-trivial.

Thanks,

/mjt

Re: [PATCH] ui/spice: Fix abort on SPICE client connect

Posted by Marc-André Lureau 6 days, 8 hours ago

Hi

On Sun, Sep 21, 2025 at 1:04 AM Mohamed Akram <mohd.akram@outlook.com> wrote:
>
> The BQL is already locked by the main loop, so avoid locking again
> unless needed to not trigger an assertion failure.
>
> Signed-off-by: Mohamed Akram <mohd.akram@outlook.com>
> ---
>  ui/spice-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 5992f9d..31c31f2 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -222,7 +222,7 @@ static void channel_event(int event, SpiceChannelEventInfo *info)
>       * thread and grab the BQL if so before calling qemu
>       * functions.
>       */
> -    bool need_lock = !qemu_thread_is_self(&me);
> +    bool need_lock = !bql_locked();

Looks good. Do you have a reproducer or a backtrace of the assertion
or is it theoretical?

>      if (need_lock) {
>          bql_lock();
>      }
> --
> 2.51.0
>
>


-- 
Marc-André Lureau

Re: [PATCH] ui/spice: Fix abort on SPICE client connect

Posted by Mohamed Akram 6 days, 1 hour ago

Hi,

> On Sep 22, 2025, at 10:39 AM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> 
> Looks good. Do you have a reproducer or a backtrace of the assertion
> or is it theoretical?
> 

Yes, this fix resolves https://gitlab.com/qemu-project/qemu/-/issues/3070.

The issue happens as soon as one connects to the SPICE port.

This is the stack trace:

Thread 4 Crashed:
0   libsystem_kernel.dylib        	       0x192a4e388 __pthread_kill + 8
1   libsystem_pthread.dylib       	       0x192a8788c pthread_kill + 296
2   libsystem_c.dylib             	       0x192990a3c abort + 124
3   libglib-2.0.0.dylib           	       0x103a597a4 g_assertion_message + 464
4   libglib-2.0.0.dylib           	       0x103a59818 g_assertion_message_expr + 116
5   qemu-system-aarch64           	       0x100d11280 bql_lock_impl + 160
6   qemu-system-aarch64           	       0x100e63c9c channel_event + 160
7   libspice-server.1.dylib       	       0x103ec5ba4 reds_handle_channel_event + 32
8   libspice-server.1.dylib       	       0x103eaa6ec DispatcherPrivate::handle_single_read() + 172
9   libspice-server.1.dylib       	       0x103eaab9c DispatcherPrivate::handle_event(int, int, DispatcherPrivate*) + 24
10  qemu-system-aarch64           	       0x100e6401c watch_read + 64
11  qemu-system-aarch64           	       0x10158da00 aio_dispatch_handler + 696
12  qemu-system-aarch64           	       0x10158c94c aio_dispatch_handlers + 256
13  qemu-system-aarch64           	       0x10158c818 aio_dispatch + 44
14  qemu-system-aarch64           	       0x1015b446c aio_ctx_dispatch + 96
15  libglib-2.0.0.dylib           	       0x103a35b0c g_main_context_dispatch_unlocked + 236
16  libglib-2.0.0.dylib           	       0x103a35a10 g_main_context_dispatch + 44
17  qemu-system-aarch64           	       0x1015b595c glib_pollfds_poll + 92
18  qemu-system-aarch64           	       0x1015b50d0 os_host_main_loop_wait + 132
19  qemu-system-aarch64           	       0x1015b4f78 main_loop_wait + 220
20  qemu-system-aarch64           	       0x100d4ae6c qemu_main_loop + 60
21  qemu-system-aarch64           	       0x101482ab4 qemu_default_main + 44
22  qemu-system-aarch64           	       0x101593fec qemu_thread_start + 216
23  libsystem_pthread.dylib       	       0x192a87c0c _pthread_start + 136
24  libsystem_pthread.dylib       	       0x192a82b80 thread_start + 8

Regards,
Mohamed