* [PATCH v3 0/3] ui/cocoa: Run qemu_init in the main thread
@ 2022-07-15 13:47 Akihiko Odaki
2022-07-15 13:47 ` [PATCH v3 1/3] " Akihiko Odaki
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Akihiko Odaki @ 2022-07-15 13:47 UTC (permalink / raw
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé, Akihiko Odaki
This work is based on:
https://patchew.org/QEMU/20220317125534.38706-1-philippe.mathieu.daude@gmail.com/
Simplify the initialization dance by running qemu_init() in the main
thread before the Cocoa event loop starts. The secondary thread only
runs only qemu_main_loop() and qemu_cleanup().
This fixes a case where addRemovableDevicesMenuItems() calls
qmp_query_block() while expecting the main thread to still hold
the BQL.
Overriding the code after calling qemu_init() is done by dynamically
replacing a function pointer variable, qemu_main when initializing
ui/cocoa, which unifies the static implementation of main() for
builds with ui/cocoa and ones without ui/cocoa.
v3: Document functions involved in startup. (Peter Maydell)
v2: Restore allow_events flag to fix the crash reported by
Philippe Mathieu-Daudé.
Akihiko Odaki (3):
ui/cocoa: Run qemu_init in the main thread
Revert "main-loop: Disable block backend global state assertion on
Cocoa"
meson: Allow to enable gtk and sdl while cocoa is enabled
docs/devel/fuzzing.rst | 4 +-
include/qemu-main.h | 3 +-
include/qemu/main-loop.h | 13 ---
include/sysemu/sysemu.h | 2 +-
meson.build | 10 +--
softmmu/main.c | 14 ++--
softmmu/vl.c | 2 +-
tests/qtest/fuzz/fuzz.c | 2 +-
ui/cocoa.m | 172 ++++++++++++++-------------------------
9 files changed, 78 insertions(+), 144 deletions(-)
--
2.32.1 (Apple Git-133)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/3] ui/cocoa: Run qemu_init in the main thread
2022-07-15 13:47 [PATCH v3 0/3] ui/cocoa: Run qemu_init in the main thread Akihiko Odaki
@ 2022-07-15 13:47 ` Akihiko Odaki
2022-07-16 10:16 ` Paolo Bonzini
2022-07-15 13:47 ` [PATCH v3 2/3] Revert "main-loop: Disable block backend global state assertion on Cocoa" Akihiko Odaki
2022-07-15 13:47 ` [PATCH v3 3/3] meson: Allow to enable gtk and sdl while cocoa is enabled Akihiko Odaki
2 siblings, 1 reply; 9+ messages in thread
From: Akihiko Odaki @ 2022-07-15 13:47 UTC (permalink / raw
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé, Akihiko Odaki
This work is based on:
https://patchew.org/QEMU/20220317125534.38706-1-philippe.mathieu.daude@gmail.com/
Simplify the initialization dance by running qemu_init() in the main
thread before the Cocoa event loop starts. The secondary thread only
runs only qemu_main_loop() and qemu_cleanup().
This fixes a case where addRemovableDevicesMenuItems() calls
qmp_query_block() while expecting the main thread to still hold
the BQL.
Overriding the code after calling qemu_init() is done by dynamically
replacing a function pointer variable, qemu_main when initializing
ui/cocoa, which unifies the static implementation of main() for
builds with ui/cocoa and ones without ui/cocoa.
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
docs/devel/fuzzing.rst | 4 +-
include/qemu-main.h | 3 +-
include/sysemu/sysemu.h | 2 +-
softmmu/main.c | 14 ++--
softmmu/vl.c | 2 +-
tests/qtest/fuzz/fuzz.c | 2 +-
ui/cocoa.m | 172 +++++++++++++++-------------------------
7 files changed, 76 insertions(+), 123 deletions(-)
diff --git a/docs/devel/fuzzing.rst b/docs/devel/fuzzing.rst
index 784ecb99e66..715330c8561 100644
--- a/docs/devel/fuzzing.rst
+++ b/docs/devel/fuzzing.rst
@@ -287,8 +287,8 @@ select the fuzz target. Then, the qtest client is initialized. If the target
requires qos, qgraph is set up and the QOM/LIBQOS modules are initialized.
Then the QGraph is walked and the QEMU cmd_line is determined and saved.
-After this, the ``vl.c:qemu_main`` is called to set up the guest. There are
-target-specific hooks that can be called before and after qemu_main, for
+After this, the ``vl.c:main`` is called to set up the guest. There are
+target-specific hooks that can be called before and after main, for
additional setup(e.g. PCI setup, or VM snapshotting).
``LLVMFuzzerTestOneInput``: Uses qtest/qos functions to act based on the fuzz
diff --git a/include/qemu-main.h b/include/qemu-main.h
index 6a3e90d0ad5..6889375e7c2 100644
--- a/include/qemu-main.h
+++ b/include/qemu-main.h
@@ -5,6 +5,7 @@
#ifndef QEMU_MAIN_H
#define QEMU_MAIN_H
-int qemu_main(int argc, char **argv, char **envp);
+void qemu_default_main(void);
+extern void (*qemu_main)(void);
#endif /* QEMU_MAIN_H */
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 812f66a31a9..254c1eabf57 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -102,7 +102,7 @@ void qemu_boot_set(const char *boot_order, Error **errp);
bool defaults_enabled(void);
-void qemu_init(int argc, char **argv, char **envp);
+void qemu_init(int argc, char **argv);
void qemu_main_loop(void);
void qemu_cleanup(void);
diff --git a/softmmu/main.c b/softmmu/main.c
index c00432ff098..41a091f2c72 100644
--- a/softmmu/main.c
+++ b/softmmu/main.c
@@ -30,18 +30,18 @@
#include <SDL.h>
#endif
-int qemu_main(int argc, char **argv, char **envp)
+void qemu_default_main(void)
{
- qemu_init(argc, argv, envp);
qemu_main_loop();
qemu_cleanup();
-
- return 0;
}
-#ifndef CONFIG_COCOA
+void (*qemu_main)(void) = qemu_default_main;
+
int main(int argc, char **argv)
{
- return qemu_main(argc, argv, NULL);
+ qemu_init(argc, argv);
+ qemu_main();
+
+ return 0;
}
-#endif
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 3f264d4b093..e8c73d0bb40 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2589,7 +2589,7 @@ void qmp_x_exit_preconfig(Error **errp)
}
}
-void qemu_init(int argc, char **argv, char **envp)
+void qemu_init(int argc, char **argv)
{
QemuOpts *opts;
QemuOpts *icount_opts = NULL, *accel_opts = NULL;
diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index 0ad4ba9e94d..678c312923a 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -236,7 +236,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
g_free(pretty_cmd_line);
}
- qemu_init(result.we_wordc, result.we_wordv, NULL);
+ qemu_init(result.we_wordc, result.we_wordv);
/* re-enable the rcu atfork, which was previously disabled in qemu_init */
rcu_enable_atfork();
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 6a4dccff7f0..c181d8d2fb3 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -100,13 +100,11 @@ static void cocoa_switch(DisplayChangeListener *dcl,
static int left_command_key_enabled = 1;
static bool swap_opt_cmd;
-static int gArgc;
-static char **gArgv;
+static QemuThread qemu_main_thread;
+static bool qemu_main_terminating;
static bool stretch_video;
static NSTextField *pauseLabel;
-static QemuSemaphore display_init_sem;
-static QemuSemaphore app_started_sem;
static bool allow_events;
static NSInteger cbchangecount = -1;
@@ -585,7 +583,7 @@ - (void) updateUIInfo
/*
* Don't try to tell QEMU about UI information in the application
* startup phase -- we haven't yet registered dcl with the QEMU UI
- * layer, and also trying to take the iothread lock would deadlock.
+ * layer.
* When cocoa_display_init() does register the dcl, the UI layer
* will call cocoa_switch(), which will call updateUIInfo, so
* we don't lose any information here.
@@ -778,16 +776,6 @@ - (void) handleMonitorInput:(NSEvent *)event
- (bool) handleEvent:(NSEvent *)event
{
- if(!allow_events) {
- /*
- * Just let OSX have all events that arrive before
- * applicationDidFinishLaunching.
- * This avoids a deadlock on the iothread lock, which cocoa_display_init()
- * will not drop until after the app_started_sem is posted. (In theory
- * there should not be any such events, but OSX Catalina now emits some.)
- */
- return false;
- }
return bool_with_iothread_lock(^{
return [self handleEventLocked:event];
});
@@ -1283,8 +1271,6 @@ - (void)applicationDidFinishLaunching: (NSNotification *) note
{
COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
allow_events = true;
- /* Tell cocoa_display_init to proceed */
- qemu_sem_post(&app_started_sem);
}
- (void)applicationWillTerminate:(NSNotification *)aNotification
@@ -1292,16 +1278,13 @@ - (void)applicationWillTerminate:(NSNotification *)aNotification
COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");
with_iothread_lock(^{
- shutdown_action = SHUTDOWN_ACTION_POWEROFF;
- qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+ if (!qemu_main_terminating) {
+ shutdown_action = SHUTDOWN_ACTION_POWEROFF;
+ qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+ }
});
- /*
- * Sleep here, because returning will cause OSX to kill us
- * immediately; the QEMU main loop will handle the shutdown
- * request and terminate the process.
- */
- [NSThread sleepForTimeInterval:INFINITY];
+ qemu_thread_join(&qemu_main_thread);
}
- (BOOL)applicationShouldTerminateAfterLastWindowClosed:(NSApplication *)theApplication
@@ -1313,7 +1296,7 @@ - (NSApplicationTerminateReply)applicationShouldTerminate:
(NSApplication *)sender
{
COCOA_DEBUG("QemuCocoaAppController: applicationShouldTerminate\n");
- return [self verifyQuit];
+ return qatomic_read(&qemu_main_terminating) || [self verifyQuit];
}
- (void)windowDidChangeScreen:(NSNotification *)notification
@@ -1915,92 +1898,42 @@ static void cocoa_clipboard_request(QemuClipboardInfo *info,
/*
* The startup process for the OSX/Cocoa UI is complicated, because
* OSX insists that the UI runs on the initial main thread, and so we
- * need to start a second thread which runs the vl.c qemu_main():
- *
- * Initial thread: 2nd thread:
+ * need to start a second thread which runs the qemu_default_main():
* in main():
- * create qemu-main thread
- * wait on display_init semaphore
- * call qemu_main()
- * ...
- * in cocoa_display_init():
- * post the display_init semaphore
- * wait on app_started semaphore
- * create application, menus, etc
- * enter OSX run loop
- * in applicationDidFinishLaunching:
- * post app_started semaphore
- * tell main thread to fullscreen if needed
- * [...]
- * run qemu main-loop
- *
- * We do this in two stages so that we don't do the creation of the
- * GUI application menus and so on for command line options like --help
- * where we want to just print text to stdout and exit immediately.
+ * in cocoa_display_init():
+ * assign cocoa_main to qemu_main
+ * create application, menus, etc
+ * in cocoa_main():
+ * create qemu-main thread
+ * enter OSX run loop
*/
static void *call_qemu_main(void *opaque)
{
- int status;
-
- COCOA_DEBUG("Second thread: calling qemu_main()\n");
- status = qemu_main(gArgc, gArgv, *_NSGetEnviron());
- COCOA_DEBUG("Second thread: qemu_main() returned, exiting\n");
+ COCOA_DEBUG("Second thread: calling qemu_default_main()\n");
+ qemu_mutex_lock_iothread();
+ qemu_default_main();
+ qatomic_set(&qemu_main_terminating, true);
+ qemu_mutex_unlock_iothread();
+ COCOA_DEBUG("Second thread: qemu_default_main() returned, exiting\n");
[cbowner release];
- exit(status);
-}
-
-int main (int argc, char **argv) {
- QemuThread thread;
-
- COCOA_DEBUG("Entered main()\n");
- gArgc = argc;
- gArgv = argv;
-
- qemu_sem_init(&display_init_sem, 0);
- qemu_sem_init(&app_started_sem, 0);
-
- qemu_thread_create(&thread, "qemu_main", call_qemu_main,
- NULL, QEMU_THREAD_DETACHED);
+ [NSApp terminate:nil];
- COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
- qemu_sem_wait(&display_init_sem);
- COCOA_DEBUG("Main thread: initializing app\n");
-
- NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
-
- // Pull this console process up to being a fully-fledged graphical
- // app with a menubar and Dock icon
- ProcessSerialNumber psn = { 0, kCurrentProcess };
- TransformProcessType(&psn, kProcessTransformToForegroundApplication);
-
- [QemuApplication sharedApplication];
-
- create_initial_menus();
+ return NULL;
+}
- /*
- * Create the menu entries which depend on QEMU state (for consoles
- * and removeable devices). These make calls back into QEMU functions,
- * which is OK because at this point we know that the second thread
- * holds the iothread lock and is synchronously waiting for us to
- * finish.
- */
- add_console_menu_entries();
- addRemovableDevicesMenuItems();
+static void cocoa_main()
+{
+ COCOA_DEBUG("Entered %s()\n", __func__);
- // Create an Application controller
- QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
- [NSApp setDelegate:appController];
+ qemu_mutex_unlock_iothread();
+ qemu_thread_create(&qemu_main_thread, "qemu_main", call_qemu_main,
+ NULL, QEMU_THREAD_JOINABLE);
// Start the main event loop
COCOA_DEBUG("Main thread: entering OSX run loop\n");
[NSApp run];
COCOA_DEBUG("Main thread: left OSX run loop, exiting\n");
-
- [appController release];
- [pool release];
-
- return 0;
}
@@ -2079,25 +2012,42 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
{
+ NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+
COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
- /* Tell main thread to go ahead and create the app and enter the run loop */
- qemu_sem_post(&display_init_sem);
- qemu_sem_wait(&app_started_sem);
- COCOA_DEBUG("cocoa_display_init: app start completed\n");
+ qemu_main = cocoa_main;
+
+ // Pull this console process up to being a fully-fledged graphical
+ // app with a menubar and Dock icon
+ ProcessSerialNumber psn = { 0, kCurrentProcess };
+ TransformProcessType(&psn, kProcessTransformToForegroundApplication);
+
+ [QemuApplication sharedApplication];
+
+ create_initial_menus();
+
+ /*
+ * Create the menu entries which depend on QEMU state (for consoles
+ * and removeable devices). These make calls back into QEMU functions,
+ * which is OK because at this point we know that the second thread
+ * holds the iothread lock and is synchronously waiting for us to
+ * finish.
+ */
+ add_console_menu_entries();
+ addRemovableDevicesMenuItems();
+
+ // Create an Application controller
+ QemuCocoaAppController *controller = [[QemuCocoaAppController alloc] init];
+ [NSApp setDelegate:controller];
- QemuCocoaAppController *controller = (QemuCocoaAppController *)[[NSApplication sharedApplication] delegate];
/* if fullscreen mode is to be used */
if (opts->has_full_screen && opts->full_screen) {
- dispatch_async(dispatch_get_main_queue(), ^{
- [NSApp activateIgnoringOtherApps: YES];
- [controller toggleFullScreen: nil];
- });
+ [NSApp activateIgnoringOtherApps: YES];
+ [controller toggleFullScreen: nil];
}
if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) {
- dispatch_async(dispatch_get_main_queue(), ^{
- [controller setFullGrab: nil];
- });
+ [controller setFullGrab: nil];
}
if (opts->has_show_cursor && opts->show_cursor) {
@@ -2117,6 +2067,8 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
qemu_event_init(&cbevent, false);
cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
qemu_clipboard_peer_register(&cbpeer);
+
+ [pool release];
}
static QemuDisplay qemu_display_cocoa = {
--
2.32.1 (Apple Git-133)
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] Revert "main-loop: Disable block backend global state assertion on Cocoa"
2022-07-15 13:47 [PATCH v3 0/3] ui/cocoa: Run qemu_init in the main thread Akihiko Odaki
2022-07-15 13:47 ` [PATCH v3 1/3] " Akihiko Odaki
@ 2022-07-15 13:47 ` Akihiko Odaki
2022-07-15 13:47 ` [PATCH v3 3/3] meson: Allow to enable gtk and sdl while cocoa is enabled Akihiko Odaki
2 siblings, 0 replies; 9+ messages in thread
From: Akihiko Odaki @ 2022-07-15 13:47 UTC (permalink / raw
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé, Akihiko Odaki
This reverts commit 47281859f66bdab1974fb122cab2cbb4a1c9af7f.
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
include/qemu/main-loop.h | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 5518845299d..0aa36a4f17e 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -280,23 +280,10 @@ bool qemu_mutex_iothread_locked(void);
bool qemu_in_main_thread(void);
/* Mark and check that the function is part of the global state API. */
-#ifdef CONFIG_COCOA
-/*
- * When using the Cocoa UI, addRemovableDevicesMenuItems() is called from
- * a thread different from the QEMU main thread and can not take the BQL,
- * triggering this assertions in the block layer (commit 0439c5a462).
- * As the Cocoa fix is not trivial, disable this assertion for the v7.0.0
- * release (when using Cocoa); we will restore it immediately after the
- * release.
- * This issue is tracked as https://gitlab.com/qemu-project/qemu/-/issues/926
- */
-#define GLOBAL_STATE_CODE()
-#else
#define GLOBAL_STATE_CODE() \
do { \
assert(qemu_in_main_thread()); \
} while (0)
-#endif /* CONFIG_COCOA */
/* Mark and check that the function is part of the I/O API. */
#define IO_CODE() \
--
2.32.1 (Apple Git-133)
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] meson: Allow to enable gtk and sdl while cocoa is enabled
2022-07-15 13:47 [PATCH v3 0/3] ui/cocoa: Run qemu_init in the main thread Akihiko Odaki
2022-07-15 13:47 ` [PATCH v3 1/3] " Akihiko Odaki
2022-07-15 13:47 ` [PATCH v3 2/3] Revert "main-loop: Disable block backend global state assertion on Cocoa" Akihiko Odaki
@ 2022-07-15 13:47 ` Akihiko Odaki
2 siblings, 0 replies; 9+ messages in thread
From: Akihiko Odaki @ 2022-07-15 13:47 UTC (permalink / raw
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé, Akihiko Odaki
As ui/cocoa does no longer override main(), ui/gtk and ui/sdl
can be enabled even ui/cocoa is enabled.
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
meson.build | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/meson.build b/meson.build
index bc5569ace15..7baec7896ef 100644
--- a/meson.build
+++ b/meson.build
@@ -583,12 +583,6 @@ if get_option('attr').allowed()
endif
cocoa = dependency('appleframeworks', modules: 'Cocoa', required: get_option('cocoa'))
-if cocoa.found() and get_option('sdl').enabled()
- error('Cocoa and SDL cannot be enabled at the same time')
-endif
-if cocoa.found() and get_option('gtk').enabled()
- error('Cocoa and GTK+ cannot be enabled at the same time')
-endif
vmnet = dependency('appleframeworks', modules: 'vmnet', required: get_option('vmnet'))
if vmnet.found() and not cc.has_header_symbol('vmnet/vmnet.h',
@@ -915,7 +909,7 @@ if not get_option('brlapi').auto() or have_system
endif
sdl = not_found
-if not get_option('sdl').auto() or (have_system and not cocoa.found())
+if not get_option('sdl').auto() or have_system
sdl = dependency('sdl2', required: get_option('sdl'), kwargs: static_kwargs)
sdl_image = not_found
endif
@@ -1181,7 +1175,7 @@ endif
gtk = not_found
gtkx11 = not_found
vte = not_found
-if not get_option('gtk').auto() or (have_system and not cocoa.found())
+if not get_option('gtk').auto() or have_system
gtk = dependency('gtk+-3.0', version: '>=3.22.0',
method: 'pkg-config',
required: get_option('gtk'),
--
2.32.1 (Apple Git-133)
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] ui/cocoa: Run qemu_init in the main thread
2022-07-15 13:47 ` [PATCH v3 1/3] " Akihiko Odaki
@ 2022-07-16 10:16 ` Paolo Bonzini
2022-07-16 11:30 ` Akihiko Odaki
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2022-07-16 10:16 UTC (permalink / raw
To: Akihiko Odaki
Cc: qemu-devel, Peter Maydell, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé
On 7/15/22 15:47, Akihiko Odaki wrote:
> static void *call_qemu_main(void *opaque)
> {
> ...
> + [NSApp terminate:nil];
>
Is this thread-safe? I think it should be something like:
- here:
/*
* Nothing more to do in the QEMU thread, ask the application
* to exit.
*/
dispatch_async(dispatch_get_main_queue(), ^{
[NSApp terminate: nil];
});
- in verifyQuit:
if([alert runModal] == NSAlertSecondButtonReturn) {
with_iothread_lock(^{
shutdown_action = SHUTDOWN_ACTION_POWEROFF;
qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
});
}
/* and just return void */
- applicationShouldTerminate: should be just
if (qatomic_read(&qemu_main_terminating)) {
return NSTerminateNow;
} else {
dispatch_async(dispatch_get_main_queue(), ^{
[self verifyQuit];
});
}
return NSTerminateCancel;
- applicationWillTerminate: can be just the qemu_thread_join
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] ui/cocoa: Run qemu_init in the main thread
2022-07-16 10:16 ` Paolo Bonzini
@ 2022-07-16 11:30 ` Akihiko Odaki
2022-07-16 16:39 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Akihiko Odaki @ 2022-07-16 11:30 UTC (permalink / raw
To: Paolo Bonzini
Cc: qemu-devel, Peter Maydell, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé
On 2022/07/16 19:16, Paolo Bonzini wrote:
> On 7/15/22 15:47, Akihiko Odaki wrote:
>> static void *call_qemu_main(void *opaque)
>> {
>> ...
>> + [NSApp terminate:nil];
> Is this thread-safe? I think it should be something like:
Yes, -[NSApplication terminate:] is thread-safe. Methods of
NSApplication handle dispatches to the main thread by themselves.
Regards,
Akihiko Odaki
>
> - here:
>
> /*
> * Nothing more to do in the QEMU thread, ask the application
> * to exit.
> */
> dispatch_async(dispatch_get_main_queue(), ^{
> [NSApp terminate: nil];
> });
>
> - in verifyQuit:
>
> if([alert runModal] == NSAlertSecondButtonReturn) {
> with_iothread_lock(^{
> shutdown_action = SHUTDOWN_ACTION_POWEROFF;
> qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
> });
> }
> /* and just return void */
>
> - applicationShouldTerminate: should be just
>
> if (qatomic_read(&qemu_main_terminating)) {
> return NSTerminateNow;
> } else {
> dispatch_async(dispatch_get_main_queue(), ^{
> [self verifyQuit];
> });
> }
> return NSTerminateCancel;
>
> - applicationWillTerminate: can be just the qemu_thread_join
>
> Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] ui/cocoa: Run qemu_init in the main thread
2022-07-16 11:30 ` Akihiko Odaki
@ 2022-07-16 16:39 ` Paolo Bonzini
2022-07-16 20:41 ` Akihiko Odaki
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2022-07-16 16:39 UTC (permalink / raw
To: Akihiko Odaki
Cc: qemu-devel, Peter Maydell, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 1503 bytes --]
Il sab 16 lug 2022, 13:30 Akihiko Odaki <akihiko.odaki@gmail.com> ha
scritto:
> On 2022/07/16 19:16, Paolo Bonzini wrote:
> > On 7/15/22 15:47, Akihiko Odaki wrote:
> >> static void *call_qemu_main(void *opaque)
> >> {
> >> ...
> >> + [NSApp terminate:nil];
> > Is this thread-safe? I think it should be something like:
>
> Yes, -[NSApplication terminate:] is thread-safe. Methods of
> NSApplication handle dispatches to the main thread by themselves.
>
Is this documented anywhere?
Paolo
Regards,
> Akihiko Odaki
>
> >
> > - here:
> >
> > /*
> > * Nothing more to do in the QEMU thread, ask the application
> > * to exit.
> > */
> > dispatch_async(dispatch_get_main_queue(), ^{
> > [NSApp terminate: nil];
> > });
> >
> > - in verifyQuit:
> >
> > if([alert runModal] == NSAlertSecondButtonReturn) {
> > with_iothread_lock(^{
> > shutdown_action = SHUTDOWN_ACTION_POWEROFF;
> > qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
> > });
> > }
> > /* and just return void */
> >
> > - applicationShouldTerminate: should be just
> >
> > if (qatomic_read(&qemu_main_terminating)) {
> > return NSTerminateNow;
> > } else {
> > dispatch_async(dispatch_get_main_queue(), ^{
> > [self verifyQuit];
> > });
> > }
> > return NSTerminateCancel;
> >
> > - applicationWillTerminate: can be just the qemu_thread_join
> >
> > Paolo
>
>
[-- Attachment #2: Type: text/html, Size: 2515 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] ui/cocoa: Run qemu_init in the main thread
2022-07-16 16:39 ` Paolo Bonzini
@ 2022-07-16 20:41 ` Akihiko Odaki
2022-07-17 3:18 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Akihiko Odaki @ 2022-07-16 20:41 UTC (permalink / raw
To: Paolo Bonzini
Cc: qemu-devel, Peter Maydell, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé
On 2022/07/17 1:39, Paolo Bonzini wrote:
> Il sab 16 lug 2022, 13:30 Akihiko Odaki <akihiko.odaki@gmail.com> ha
> scritto:
>
>> On 2022/07/16 19:16, Paolo Bonzini wrote:
>>> On 7/15/22 15:47, Akihiko Odaki wrote:
>>>> static void *call_qemu_main(void *opaque)
>>>> {
>>>> ...
>>>> + [NSApp terminate:nil];
>>> Is this thread-safe? I think it should be something like:
>>
>> Yes, -[NSApplication terminate:] is thread-safe. Methods of
>> NSApplication handle dispatches to the main thread by themselves.
>>
>
> Is this documented anywhere?
Thread-unsafe classes (and more importantly, main thread only classes)
are rather exceptional, and they are listed at:
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/ThreadSafetySummary/ThreadSafetySummary.html
Regards,
Akihiko Odaki
>
> Paolo
>
> Regards,
>> Akihiko Odaki
>>
>>>
>>> - here:
>>>
>>> /*
>>> * Nothing more to do in the QEMU thread, ask the application
>>> * to exit.
>>> */
>>> dispatch_async(dispatch_get_main_queue(), ^{
>>> [NSApp terminate: nil];
>>> });
>>>
>>> - in verifyQuit:
>>>
>>> if([alert runModal] == NSAlertSecondButtonReturn) {
>>> with_iothread_lock(^{
>>> shutdown_action = SHUTDOWN_ACTION_POWEROFF;
>>> qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
>>> });
>>> }
>>> /* and just return void */
>>>
>>> - applicationShouldTerminate: should be just
>>>
>>> if (qatomic_read(&qemu_main_terminating)) {
>>> return NSTerminateNow;
>>> } else {
>>> dispatch_async(dispatch_get_main_queue(), ^{
>>> [self verifyQuit];
>>> });
>>> }
>>> return NSTerminateCancel;
>>>
>>> - applicationWillTerminate: can be just the qemu_thread_join
>>>
>>> Paolo
>>
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] ui/cocoa: Run qemu_init in the main thread
2022-07-16 20:41 ` Akihiko Odaki
@ 2022-07-17 3:18 ` Paolo Bonzini
0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2022-07-17 3:18 UTC (permalink / raw
To: Akihiko Odaki
Cc: qemu-devel, Peter Maydell, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 2320 bytes --]
Il sab 16 lug 2022, 22:42 Akihiko Odaki <akihiko.odaki@gmail.com> ha
scritto:
> > Is this documented anywhere?
>
> Thread-unsafe classes (and more importantly, main thread only classes)
> are rather exceptional, and they are listed at:
>
> https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/ThreadSafetySummary/ThreadSafetySummary.html
Being thread-safe is not the same thing as deferring to the main thread
though.
Here are a couple of examples of how things can go wrong, depending on the
implementation of Cocoa:
- applicationWillTerminate is a notification, and as such will run in the
main thread, but will applicationShouldTerminate also be called only from
the main thread? If not, is it safe to show the alert in verifyQuit?
- is the secondary thread still in terminate: when applicationWillTerminate
runs, and is it waiting for applicationWillTerminate to finish? If so,
qemu_thread_join will deadlock (because the waited thread will not exit
until applicationWillTerminate returns)
In both cases the problem is not thread-unsafety of Cocoa, but
thread-unsafety of the QEMU application delegate.
Paolo
> Regards,
> Akihiko Odaki
>
> >
> > Paolo
> >
> > Regards,
> >> Akihiko Odaki
> >>
> >>>
> >>> - here:
> >>>
> >>> /*
> >>> * Nothing more to do in the QEMU thread, ask the application
> >>> * to exit.
> >>> */
> >>> dispatch_async(dispatch_get_main_queue(), ^{
> >>> [NSApp terminate: nil];
> >>> });
> >>>
> >>> - in verifyQuit:
> >>>
> >>> if([alert runModal] == NSAlertSecondButtonReturn) {
> >>> with_iothread_lock(^{
> >>> shutdown_action = SHUTDOWN_ACTION_POWEROFF;
> >>> qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
> >>> });
> >>> }
> >>> /* and just return void */
> >>>
> >>> - applicationShouldTerminate: should be just
> >>>
> >>> if (qatomic_read(&qemu_main_terminating)) {
> >>> return NSTerminateNow;
> >>> } else {
> >>> dispatch_async(dispatch_get_main_queue(), ^{
> >>> [self verifyQuit];
> >>> });
> >>> }
> >>> return NSTerminateCancel;
> >>>
> >>> - applicationWillTerminate: can be just the qemu_thread_join
> >>>
> >>> Paolo
> >>
> >>
> >
>
>
[-- Attachment #2: Type: text/html, Size: 3943 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-07-17 3:19 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-15 13:47 [PATCH v3 0/3] ui/cocoa: Run qemu_init in the main thread Akihiko Odaki
2022-07-15 13:47 ` [PATCH v3 1/3] " Akihiko Odaki
2022-07-16 10:16 ` Paolo Bonzini
2022-07-16 11:30 ` Akihiko Odaki
2022-07-16 16:39 ` Paolo Bonzini
2022-07-16 20:41 ` Akihiko Odaki
2022-07-17 3:18 ` Paolo Bonzini
2022-07-15 13:47 ` [PATCH v3 2/3] Revert "main-loop: Disable block backend global state assertion on Cocoa" Akihiko Odaki
2022-07-15 13:47 ` [PATCH v3 3/3] meson: Allow to enable gtk and sdl while cocoa is enabled Akihiko Odaki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.