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

[Peter Maydell]

On Fri, 22 Feb 2019 at 21:48, Roman Bolshakov <address@hidden> wrote:
>
> 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 :)

It only happens for some guest workloads. The "usual" case
is that the cocoa_refresh callback is called from the QEMU
main loop, which happens to be on the OSX main thread, which
means OSX is still happy. But in some cases cocoa_refresh can
be called from a guest vCPU thread -- I think we've seen this
when a guest initiates a screen resolution change: the call
from the guest vCPU thread goes into the model of the graphics
device, which makes a call into the UI midlayer to say
"resolution changed", which immediately triggers a
refresh callback to the UI frontend layer from that thread.
In Mojave this causes OSX to terminate QEMU. I think in
older OSX versions it would probably be a race condition,
so it's technically a bug but not one that usually has
any visible bad effects; it's only surfaced as a problem
now that Mojave actively checks for this condition and
kills the process.

thanks
-- PMM