Bug 1438539 - Include conditional_wakeup fix in glib2
Summary: Include conditional_wakeup fix in glib2
Alias: None
Product: Fedora
Classification: Fedora
Component: glib2
Version: 25
Hardware: Unspecified
OS: Unspecified
Target Milestone: ---
Assignee: Matthias Clasen
QA Contact: Fedora Extras Quality Assurance
Depends On:
Blocks: TRACKER-bugs-affecting-libguestfs 1473605
TreeView+ depends on / blocked
Reported: 2017-04-03 17:29 UTC by Richard W.M. Jones
Modified: 2017-07-24 14:21 UTC (History)
13 users (show)

Fixed In Version: glib2-2.52.2-2.fc26
Doc Type: If docs needed, set a value
Doc Text:
Clone Of: 1435432
: 1473605 (view as bug list)
Last Closed: 2017-05-16 00:56:16 UTC
Type: Bug

Attachments (Terms of Use)

System ID Private Priority Status Summary Last Updated
GNOME Bugzilla 761102 0 None None None 2017-04-03 17:30:28 UTC

Description Richard 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.


** 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 (;;) {

> 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.

Comment 1 Kalev Lember 2017-05-15 09:16:27 UTC
Should be fixed in glib2-2.52.2-2.fc26

Comment 2 Fedora Update System 2017-05-15 09:18:29 UTC
glib2-2.52.2-2.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-27fbf57af3

Comment 3 Fedora Update System 2017-05-15 22:05:16 UTC
glib2-2.52.2-2.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-27fbf57af3

Comment 4 Fedora Update System 2017-05-16 00:56:16 UTC
glib2-2.52.2-2.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.