Re: [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU

[Roman Bolshakov]

On Fri, Feb 22, 2019 at 03:41:05PM +0000, Peter Maydell wrote:
> On Fri, 22 Feb 2019 at 15:19, Roman Bolshakov <address@hidden> wrote:
> >
> > On Thu, Feb 14, 2019 at 10:28:10AM +0000, Peter Maydell wrote:
> > > The Cocoa UI should run on the main thread; this is enforced
> > > in OSX Mojave. In order to be able to run on the main thread,
> > > we need to make sure we hold the iothread lock whenever we
> > > call into various QEMU UI midlayer functions.
> > >
> >
> > I also think it's better to clarify that the reason of the commit is not
> > Mojave enforcing usage of event loop in main thread but an improvement
> > of event processing in Cocoa UI, because Cocoa UI works on Mojave.
> 
> Hmm? The point of this patchset is exactly that Mojave enforces
> that things go on the main thread, where previous OSX versions
> did not, and so in some situations QEMU will crash on Mojave
> where it did not on older versions. So I'm not sure what you're
> suggesting should be clarified here.
> 

I'm not exactly sure there's an issue with QEMU on Mojave. But I lean
towards the opinion because I haven't seen it :)

> > As of now qemu main loop and Cocoa events processing is done in the same
> > thread. And you can see from below that invocation of the qemu_main blocks
> > Cocoa event loop [NSApp run] in applicationDidFinishLaunching as qemu_main
> > doesn't quit.
> 
> Yes, indeed. This is how we've traditionally done the Cocoa
> event handling. It's worked OK for everything up to Mojave...
> 

Oh, you also mention that in the cover letter. I somehow skipped it.

> > Each millisecond cocoa_refresh gets called by display handler. The function
> > extracts all available events from the cocoa event queue and dispatches 
> > them to
> > handleEvent:
> >
> >         event = [NSApp nextEventMatchingMask:NSEventMaskAny 
> > untilDate:distantPast
> >                         inMode: NSDefaultRunLoopMode dequeue:YES];
> >         if (event != nil) {
> >             [cocoaView handleEvent:event];
> >         }
> >
> > handleEvent enqueues keyboard and mouse events into ps2 queue and the
> > other events are enqueued to Cocoa event queue. I'm curious what happens
> > to the events that are enqueued with:
> >         [NSApp sendEvent:event];
> 
> As I understand it, this sendEvent method doesn't queue anything.
> It just hands the event to the usual OSX event handling chain,
> which will either use it (for example, if it's a menu-item
> key accelerator event) or drop it on the floor. The logic is
> basically "QEMU-specific code gets first chance to decide whether
> to consume the event; the default (and a few other odd cases)
> if it does not is that we let the OSX framework code do whatever
> it wants to do with it".
> 

Thanks, I see! I should have consulted apple reference before raising
the issue. FWIW there's a method to post an event to the event queue but
sendEvent is used to dispatch an event within an app and the event that
is already taken from the queue.

Besides qmp_stop and qmp_cont not being called under iothread lock,
Reviewed-by: Roman Bolshakov <address@hidden>
Tested-by: Roman Bolshakov <address@hidden>

Thanks,
Roman