Hi Peter, On 3/4/19 5:49 PM, 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. > > Signed-off-by: Peter Maydell <[email protected]> > Reviewed-by: Roman Bolshakov <[email protected]> > Tested-by: Roman Bolshakov <[email protected]>
I got surprised by your 2 Message-id lines (same with other patches from this pull request): > Message-id: [email protected] ^ v3 > Message-id: [email protected] ^ v2 > --- > ui/cocoa.m | 91 ++++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 65 insertions(+), 26 deletions(-) > > diff --git a/ui/cocoa.m b/ui/cocoa.m > index e2567d69466..f1171c48654 100644 > --- a/ui/cocoa.m > +++ b/ui/cocoa.m > @@ -129,6 +129,21 @@ bool stretch_video; > NSTextField *pauseLabel; > NSArray * supportedImageFileTypes; > > +// Utility function to run specified code block with iothread lock held > +typedef void (^CodeBlock)(void); > + > +static void with_iothread_lock(CodeBlock block) > +{ > + bool locked = qemu_mutex_iothread_locked(); > + if (!locked) { > + qemu_mutex_lock_iothread(); > + } > + block(); > + if (!locked) { > + qemu_mutex_unlock_iothread(); > + } > +} > + > // Mac to QKeyCode conversion > const int mac_to_qkeycode_map[] = { > [kVK_ANSI_A] = Q_KEY_CODE_A, > @@ -306,6 +321,7 @@ static void handleAnyDeviceErrors(Error * err) > - (void) toggleFullScreen:(id)sender; > - (void) handleMonitorInput:(NSEvent *)event; > - (void) handleEvent:(NSEvent *)event; > +- (void) handleEventLocked:(NSEvent *)event; > - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled; > /* The state surrounding mouse grabbing is potentially confusing. > * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated > @@ -649,8 +665,14 @@ QemuCocoaView *cocoaView; > > - (void) handleEvent:(NSEvent *)event > { > - COCOA_DEBUG("QemuCocoaView: handleEvent\n"); > + with_iothread_lock(^{ > + [self handleEventLocked:event]; > + }); > +} > > +- (void) handleEventLocked:(NSEvent *)event > +{ > + COCOA_DEBUG("QemuCocoaView: handleEvent\n"); > int buttons = 0; > int keycode = 0; > bool mouse_event = false; > @@ -945,15 +967,18 @@ QemuCocoaView *cocoaView; > */ > - (void) raiseAllKeys > { > - int index; > const int max_index = ARRAY_SIZE(modifiers_state); > > - for (index = 0; index < max_index; index++) { > - if (modifiers_state[index]) { > - modifiers_state[index] = 0; > - qemu_input_event_send_key_qcode(dcl->con, index, false); > - } > - } > + with_iothread_lock(^{ > + int index; > + > + for (index = 0; index < max_index; index++) { > + if (modifiers_state[index]) { > + modifiers_state[index] = 0; > + qemu_input_event_send_key_qcode(dcl->con, index, false); > + } > + } > + }); > } > @end > > @@ -1178,7 +1203,9 @@ QemuCocoaView *cocoaView; > /* Pause the guest */ > - (void)pauseQEMU:(id)sender > { > - qmp_stop(NULL); > + with_iothread_lock(^{ > + qmp_stop(NULL); > + }); > [sender setEnabled: NO]; > [[[sender menu] itemWithTitle: @"Resume"] setEnabled: YES]; > [self displayPause]; > @@ -1187,7 +1214,9 @@ QemuCocoaView *cocoaView; > /* Resume running the guest operating system */ > - (void)resumeQEMU:(id) sender > { > - qmp_cont(NULL); > + with_iothread_lock(^{ > + qmp_cont(NULL); > + }); > [sender setEnabled: NO]; > [[[sender menu] itemWithTitle: @"Pause"] setEnabled: YES]; > [self removePause]; > @@ -1215,13 +1244,17 @@ QemuCocoaView *cocoaView; > /* Restarts QEMU */ > - (void)restartQEMU:(id)sender > { > - qmp_system_reset(NULL); > + with_iothread_lock(^{ > + qmp_system_reset(NULL); > + }); > } > > /* Powers down QEMU */ > - (void)powerDownQEMU:(id)sender > { > - qmp_system_powerdown(NULL); > + with_iothread_lock(^{ > + qmp_system_powerdown(NULL); > + }); > } > > /* Ejects the media. > @@ -1237,9 +1270,11 @@ QemuCocoaView *cocoaView; > return; > } > > - Error *err = NULL; > - qmp_eject(true, [drive cStringUsingEncoding: NSASCIIStringEncoding], > - false, NULL, false, false, &err); > + __block Error *err = NULL; > + with_iothread_lock(^{ > + qmp_eject(true, [drive cStringUsingEncoding: NSASCIIStringEncoding], > + false, NULL, false, false, &err); > + }); > handleAnyDeviceErrors(err); > } > > @@ -1271,16 +1306,18 @@ QemuCocoaView *cocoaView; > return; > } > > - Error *err = NULL; > - qmp_blockdev_change_medium(true, > - [drive cStringUsingEncoding: > - NSASCIIStringEncoding], > - false, NULL, > - [file cStringUsingEncoding: > - NSASCIIStringEncoding], > - true, "raw", > - false, 0, > - &err); > + __block Error *err = NULL; > + with_iothread_lock(^{ > + qmp_blockdev_change_medium(true, > + [drive cStringUsingEncoding: > + NSASCIIStringEncoding], > + false, NULL, > + [file cStringUsingEncoding: > + NSASCIIStringEncoding], > + true, "raw", > + false, 0, > + &err); > + }); > handleAnyDeviceErrors(err); > } > } > @@ -1419,7 +1456,9 @@ QemuCocoaView *cocoaView; > // get the throttle percentage > throttle_pct = [sender tag]; > > - cpu_throttle_set(throttle_pct); > + with_iothread_lock(^{ > + cpu_throttle_set(throttle_pct); > + }); > COCOA_DEBUG("cpu throttling at %d%c\n", cpu_throttle_get_percentage(), > '%'); > } > >
