Re: [Qemu-devel] (no subject)

[Programmingkid]

> On Nov 28, 2018, at 2:39 PM, Peter Maydell <address@hidden> wrote:
> 
> On Wed, 28 Nov 2018 at 01:12, John Arbuckle <address@hidden> wrote:
>> 
>> From af4497f2b161bb4165acb8eee5cae3f2a7ea2227 Mon Sep 17 00:00:00 2001
>> From: John Arbuckle <address@hidden>
>> Date: Tue, 27 Nov 2018 20:01:20 -0500
>> Subject: [PATCH] ui/cocoa.m: fix crash due to cocoa_refresh() on Mac OS 10.14
> 
> Something seems to have got the formatting of this patch email
> wrong -- it's got all this in the body and the actual Subject
> line of the email is blank.

I don't know what happened.

> 
>> Mac OS 10.14 only wants UI code to be called from the main thread. The
>> cocoa_refresh() function is called on another thread and this causes a
>> crash to take place. To fix this problem the cocoa_refresh() code is
>> called from the main thread only.
>> 
>> Signed-off-by: John Arbuckle <address@hidden>
>> ---
>> ui/cocoa.m | 59 ++++++++++++++++++++++++++++++++++-------------------------
>> 1 file changed, 34 insertions(+), 25 deletions(-)
> 
> I get a compile warning with this patch:
> /Users/pm215/src/qemu/ui/cocoa.m:1615:23: warning: instance method
> '-cocoa_refresh' not found (return type defaults to 'id')
> [-Wobjc-method-access]
>    [[NSApp delegate] cocoa_refresh];
>                      ^~~~~~~~~~~~~

This will fix the problem:

static void cocoa_refresh(DisplayChangeListener *dcl)
{
    QemuCocoaAppController *controller = (QemuCocoaAppController *)[NSApp 
delegate];
    [controller cocoa_refresh];
}

> To be honest, I'm still confused about what is causing the
> problems on Mojave. The refresh method should be being called
> on the main thread even with the code on master -- it's just
> that that is the iothread and it's running the event pumping
> code in the refresh callback rather than a standard OSX
> event loop. I'm hoping that the backtrace with symbols of
> the Mojave assertion failure will help there, since I
> can't currently see where refresh gets called from some
> non-main thread.

This might be a Mojave issue and not a QEMU issue. 
I'm on Mac OS 10.12 so I can't confirm anything.

> I also think that this patch doesn't address the problems
> of locking that I mention on the discussion thread for
> Berkus' patch. It also doesn't handle any of the other
> callbacks from QEMU to the cocoa UI -- surely we need to
> handle all of them if there is a problem here?

To answer this question I would have to know how my patch
does on Mac OS 10.14. Does it stop the crashing issue? If
this patch does fix that problem then I think sticking to
a simple solution may be the answer. The use of locks may
not be needed.

> (I have some prototype patches which I've been working
> on which address the locking problem and also make all
> the QEMU callbacks run their work on the main thread.
> I may be able get those into shape to post those next week.)

Please CC me when do release it. I will test it on Mac OS 10.12
and Mac OS 10.6.