Re: [PATCH] This patch fixes hanging up Cocoa display on macOS 10.15 (Catalina)

[Hikaru Nishida]

Thank you for your reply.

The event gets fired before applicationDidFinishLaunching is: (output of  NSLog(@"event: %@", event);)

event: NSEvent: type=Kitdefined loc=(0,1440) time=164310.0 flags=0x40 win=0x0 winNum=0 ctxt=0x0 subtype=1 data1=834 data2=0

I moved allow_events = true just before qemu_sem_post(&app_started_sem) and it also works.

Hikaru Nishida

2019年10月14日(月) 22:19 Peter Maydell <address@hidden>:

On Mon, 14 Oct 2019 at 13:53, <address@hidden> wrote:
>
> From: Hikaru Nishida <address@hidden>
>
> An NSEvent is fired before applicationDidFinishLaunching on macOS
> Catalina. This causes deadlock of iothread_lock by calling
> bool_with_iothread_lock in handleEvent while its already locked.
> This patch prevents to call bool_with_iothread_lock until the
> app_started_sem is released to prevent this deadlock.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1847906
> Signed-off-by: Hikaru Nishida <address@hidden>
> ---
>  ui/cocoa.m | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index f12e21df6e..f16d341a0a 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -134,6 +134,7 @@
>
>  static QemuSemaphore display_init_sem;
>  static QemuSemaphore app_started_sem;
> +volatile sig_atomic_t allow_events;
>
>  // Utility functions to run specified code block with iothread lock held
>  typedef void (^CodeBlock)(void);
> @@ -729,6 +730,9 @@ - (void) handleMonitorInput:(NSEvent *)event
>
>  - (bool) handleEvent:(NSEvent *)event
>  {
> +    if(!allow_events) {
> +        return false;
> +    }
>      return bool_with_iothread_lock(^{
>          return [self handleEventLocked:event];
>      });
> @@ -1897,6 +1901,7 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>      /* Tell main thread to go ahead and create the app and enter the run loop */
>      qemu_sem_post(&display_init_sem);
>      qemu_sem_wait(&app_started_sem);
> +    allow_events = true;
>      COCOA_DEBUG("cocoa_display_init: app start completed\n");

If we do use a flag to fix this race, I think it would be
better to set allow_events = true in the
applicationDidFinishLaunching() function before posting
the app_started_sem. Otherwise there's a window after
applicationDidFinishLaunching() returns but before
cocoa_display_init() resumes and sets the flag where we
will unnecessarily drop events.

Could you try that and check that it still fixes the hang?

thanks
-- PMM