Re: [Qemu-devel] [PATCH] Fix for crashes and non-responsive UI on macOS Mojave

[Peter Maydell]

On Thu, 22 Nov 2018 at 07:32, Gerd Hoffmann <address@hidden> wrote:
>
>   Hi,
>
> > Something somewhere in this call chain should have taken
> > the iothread lock, I assume, but I'm not sure where.
> >
> > Gerd -- what are the rules of the UI subsystem regarding
> > multiple threads, and what threads are allowed to call
> > UI functions like qemu_input_event_send_key_qcode(),
> > from which threads, and whether they need to eg hold
> > the iothread lock when they do so? There's no
> > documentation on these functions in include/ui/input.h.
>
> UI event handling code typically runs in iothread context.  So, yes,
> when calling qemu_input_* the UI code holds the iothread lock.
>
> > (To fix a problem on OSX Mojave this patchset has moved
> > which thread the UI executes on, so it is now always
> > the main thread and not the thread which calls
> > the QemuDisplay 'init' callback. That seems to break
> > an implicit assumption in the UI layer.)
>
> Hmm, I guess the options are (a) grab the iothread lock before calling
> input layer functions, or (b) queue the event and schedule a bottom half
> which processes the queue (which will then be called from iothread
> context, with the lock already taken).

I was thinking about this today, and I realized that if
we just make the OSX UI thread code grab the iothread lock,
then there is a potential deadlock with the changes (also
in this patchset) which defer the DisplayChangeListener
update/switch/refresh operations to the UI thread. The
current patchset has those be synchronous deferrals, so
you could get a deadlock if the UI thread is waiting
for the iothread lock, but the code in the UI midlayer
is holding the iothread lock and waiting for a DCL
operation to be run on the main thread.

What are the required semantics for update/switch/refresh?
These don't seem to be documented. Can we validly make
those be asychronous, or does the midlayer require that
the operation has completed and been reflected onscreen
before the update/switch/refresh callback returns ?

If those have to be synchronous, then we'll need to do
something more complicated (eg the queuing of events
that you suggest), which I'd prefer it if we can avoid,
because that implies memory allocation or a fixed
length queue (plus some fairly significant restructuring
of the cocoa frontend).

thanks
-- PMM