Created attachment 246331 [details] [review]
tests: Add a test for a specific invariant

See the documentation in the test. The invariant is used by
gnome-test-runner.

I'm totally OK with this guarantee.  If someone has explicitly called wakeup() then this is a pretty strong hint that we ought to do so.

How about docs in _wakeup()?

They currently say

"""

If context is currently waiting in a poll(), interrupt the poll(), and continue the iteration process.

"""


which is just not true....

I'm currently struggling about what guarantee to give in the docs. Because what we really want to say is "will make sure (the internal function) g_main_context_poll() will stop blocking and not block again until g_main_context_check() is called the next time". Which is roughly as descriptive as "some magic happens here".

The problem is that g_main_context_check() is changing if g_main_context_iteration() will block currently. (Is that even how it's supposed to be?)

So what guarantee do you want to give in the docs? In particular because the concept of poll()ing is kinda not what we do anyway, so talking about it seems completely wrong.

I think the guarantee needs to be in terms of which specific behaviour we define as being... defined.

In particular, I think that you should mention two things:

(a) calling g_main_context_wakeup() will cause the next g_main_context_ieration() to return without blocking.

(b) this pattern:

thread() {
  done = TRUE;
  g_main_context_wakeup();
}

while (!done)
  g_main_context_iteration(, TRUE);


is guaranteed race-free.

(In reply to comment #4)
> I think the guarantee needs to be in terms of which specific behaviour we
> define as being... defined.
> 
> In particular, I think that you should mention two things:
> 
> (a) calling g_main_context_wakeup() will cause the next
> g_main_context_ieration() to return without blocking.

That seems pretty clear.

> is guaranteed race-free.

As long as the assignment to "done" is made visible in other threads via GMutex or g_atomic_()

Created attachment 246422 [details] [review]
0001-gmain-Document-more-use-cases-of-g_main_context_wake.patch

Review of attachment 246422 [details] [review]:

::: glib/gmain.c
@@ +4333,3 @@
+ *
+ *   g_atomic_int_inc (&tasks_run);
+ *   if (g_atomic_int_get (&tasks_run) == NUM_TASKS)

There is a semi-race here: two threads might end up calling wakeup() :)

You want to implement it with g_atomic_int_dec_and_test() and a remaining_tasks int I suppose. For clarity!

Created attachment 246431 [details] [review]
updated for comments

Comment on attachment 246422 [details] [review]
0001-gmain-Document-more-use-cases-of-g_main_context_wake.patch

>+ * If @context is currently waiting in g_main_context_iteration(),
>+ * cause it to return.  Otherwise, cause the next invocation of
>+ * g_main_context_iteration() to return.

Well, g_main_context_iteration() already returns. You need to say "return immediately" or something. Except that that suggests that it won't even dispatch other sources that are ready, which isn't true. I'd just leave it in terms of poll():

  If @context is waiting in a poll(), interrupt the poll() and
  continue the iteration process. If @context is not currently
  waiting in a poll(), this will cause its next poll() to
  return immediately.

>+ * This API useful

missing "is"

Another important use is: if you implement your own GSource type which is not based on fds or timeouts, you probably need to call this to wake up the context when your source becomes ready.

I talked about this with Ryan yesterday. I don't like using poll() in the docs because (a) we might not even poll at all and (b) you'd need to know what a poll() is to understand it. The thing I would say is probably "will not wait":

  If @context is currently waiting in g_main_context_iteration(),
  cause it to return.  Otherwise, cause the next invocation of
  g_main_context_iteration() to not wait.

I might use "block" instead of "wait" because the argument to g_main_context_iteration() is called may_block:

  If @context is currently blocking in g_main_context_iteration(),
  cause it to return.  Otherwise, cause the next invocation of
  g_main_context_iteration() to not block.

Sure. Possibly even "If @context is currently blocking in g_main_context_iteration() waiting for a source to become ready, cause it to stop blocking and return."

Pushed the two coimmits with the agreed-on changes.