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
(resending in plaintext)


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