DescriptionRichard W.M. Jones
2017-04-03 17:29:06 UTC
+++ This bug was initially created as a clone of Bug #1435432 +++
Description of problem:
Please include the further fix for the glib2 possible bug/regression which
affected the qemu main loop.
https://bugzilla.gnome.org/show_bug.cgi?id=761102#c19
** Comment 19 Paolo Bonzini 2017-03-31 20:08:37 UTC
> The docs are pretty clear on this.
Actually, now that I read them again, they aren't clear at all.
They say "You must be the owner of a context before you can call g_main_context_prepare(), g_main_context_query(), g_main_context_check(), g_main_context_dispatch()." They do not say that you need to be the owner of a context between those calls.
In particular, if you release the context after g_main_context_query returns and reacquire it before g_main_context_check, it would cause the bug again:
thread A thread B
------------ ------------
for (;;) {
acquire
prepare
query
release
ppoll()
g_source_attach
acquire
check
dispatch
release
}
> Paolo, we could consider carrying a revert of this in downstream locations
> where it's affecting qemu, if it's hard to get qemu changed.
No, it's easy and we're testing the fix over the weekend. Once it's tested I'll also backport it to Fedora 25. I'm worried more about the endless amount of forks (including the Android emulator) and about the above imprecision in the document.
However I wonder if it's possible to change the patch so that it does the wakeup if no one has acquired the context. This should still provide the optimization for the users that are doing g_main_context_iteration in a loop, and it would fix both QEMU and the hypothetical case at the top of this comment.
For example something as simple as this:
- if (context->owner && context->owner != G_THREAD_SELF)
+ if (context->owner != G_THREAD_SELF)
g_wakeup_signal (context->wakeup);
(for all three occurrences) should do it.
** Comment 20 Colin Walters 2017-03-31 20:21:45 UTC
Created attachment 349077[details] [review]
Create a helper function for owner wakeup optimization
Just a quick preparatory patch here.
** Comment 21 Paolo Bonzini 2017-04-03 17:17:59 UTC
Review of attachment 349077[details] [review]:
Looks good.