On Sat, Feb 5, 2022 at 1:19 AM Peter Maydell <[email protected]> wrote: > > On Wed, 16 Jun 2021 at 15:19, Akihiko Odaki <[email protected]> wrote: > > > > Signed-off-by: Akihiko Odaki <[email protected]> > > --- > > ui/cocoa.m | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 49 insertions(+) > > Hi; I was looking at the cocoa.m code to see about maybe deleting the > unnecessary autorelease pools, and I ran into some code I was a bit > unsure about that was added in this patch. > > In particular I'm wondering about the interactions across threads here. > > > +- (void) updateUIInfo > > +{ > > + NSSize frameSize; > > + QemuUIInfo info; > > + > > + if (!qemu_console_is_graphic(dcl.con)) { > > + return; > > + } > > + > > + if ([self window]) { > > + NSDictionary *description = [[[self window] screen] > > deviceDescription]; > > + CGDirectDisplayID display = [[description > > objectForKey:@"NSScreenNumber"] unsignedIntValue]; > > + NSSize screenSize = [[[self window] screen] frame].size; > > + CGSize screenPhysicalSize = CGDisplayScreenSize(display); > > + > > + frameSize = isFullscreen ? screenSize : [self frame].size; > > + info.width_mm = frameSize.width / screenSize.width * > > screenPhysicalSize.width; > > + info.height_mm = frameSize.height / screenSize.height * > > screenPhysicalSize.height; > > + } else { > > + frameSize = [self frame].size; > > + info.width_mm = 0; > > + info.height_mm = 0; > > + } > > + > > + info.xoff = 0; > > + info.yoff = 0; > > + info.width = frameSize.width; > > + info.height = frameSize.height; > > + > > + dpy_set_ui_info(dcl.con, &info); > > This function makes some cocoa calls, and it also calls a QEMU > UI layer function, dpy_set_ui_info(). > > > +- (void)windowDidChangeScreen:(NSNotification *)notification > > +{ > > + [cocoaView updateUIInfo]; > > We call it from the cocoa UI thread in callbacks like this. > > > /* Called when the user clicks on a window's close button */ > > - (BOOL)windowShouldClose:(id)sender > > { > > @@ -1936,6 +1983,8 @@ static void cocoa_switch(DisplayChangeListener *dcl, > > > > COCOA_DEBUG("qemu_cocoa: cocoa_switch\n"); > > > > + [cocoaView updateUIInfo]; > > We also call it from the QEMU thread, when the QEMU thread calls > this cocoa_switch callback function. > > (1) A question for Akihiko: > Are all the cocoa calls we make in updateUIInfo safe to > make from a non-UI thread? Would it be preferable for this > call in cocoa_switch() to be moved inside the dispatch_async block? > (Moving it would mean that I wouldn't have to think about whether > any of the code in it needs to have an autorelease pool :-))
It is not safe. Apparently I totally forgot about threads when I wrote this. It should be moved in the dispatch_async block as you suggest. Should I write a patch, or will you write one before you delete autorelease pools? Regards, Akihiko Odaki > > (2) A question for Gerd: > Is it safe to call dpy_set_ui_info() from a non-QEMU-main-thread? > It doesn't appear to do any locking that would protect against > multiple threads calling it simultaneously. > > thanks > -- PMM
