After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 689393 - Add API to synchronize on worker threads
Add API to synchronize on worker threads
Status: RESOLVED WONTFIX
Product: glib
Classification: Platform
Component: mainloop
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2012-11-30 23:42 UTC by Giovanni Campagna
Modified: 2013-02-22 14:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GTask: add g_task_wait_sync() for synchronizing with worker threads (5.13 KB, patch)
2012-11-30 23:43 UTC, Giovanni Campagna
none Details | Review
GTask: add g_task_wait_sync() for synchronizing with worker threads (5.19 KB, patch)
2013-01-02 18:09 UTC, Giovanni Campagna
reviewed Details | Review

Description Giovanni Campagna 2012-11-30 23:42:49 UTC
Sometimes you decide to launch a slow operation in background, knowing that at some point in the future you'll need the result, but not right now.
Then that point in the future arrives: what do you do?
You need to check if the async task completed, and if not, wait for it and get the result without going back to the mainloop.

This is already kind of possible without specific API in GTask, but it's racy if not done right. That's why I think that g_task_wait_sync() is a worthy addition for glib.

PS: the specific use case for this is bug 682427, where I want to do IO and rendering in a worker thread (because GnomeBG is completely synchronous), but then I also need to have something to draw when clutter asks, so for the first frame I wait until the worker is done.
Comment 1 Giovanni Campagna 2012-11-30 23:43:16 UTC
Created attachment 230359 [details] [review]
GTask: add g_task_wait_sync() for synchronizing with worker threads

This new API allows the mainloop to wait for a background task, in
case for whatever reason the result is needed now, rather than a
later time.
Comment 2 Matthias Clasen 2012-12-03 05:04:18 UTC
Review of attachment 230359 [details] [review]:

::: gio/gtask.c
@@ +1388,3 @@
+ * during a previous iteration of the mainloop or within this function.
+ *
+ * It is only legal to call this function from the same thread that

I don't like the use of 'legal' in documentation. Better to say 'It is a programming error' or 'You must'.
Comment 3 Giovanni Campagna 2012-12-10 23:05:48 UTC
Ok, I corrected the docs locally.
Shall I attach an updated patch?
Comment 4 Matthias Clasen 2012-12-10 23:51:52 UTC
Would be nice to get a review of the general idea from Dan or Colin
Comment 5 Dan Winship 2012-12-11 09:02:46 UTC
The general idea makes sense; we might end up wanting to ease some of the restrictions that are currently on that function, but removing restrictions can be done later on without breaking abi.
Comment 6 Colin Walters 2012-12-11 15:06:05 UTC
In Java, this is Future<>.  Maybe worth looking at what they've done.
Comment 7 Giovanni Campagna 2012-12-13 23:27:52 UTC
(In reply to comment #6)
> In Java, this is Future<>.  Maybe worth looking at what they've done.

Uhm... I'm not sure I understand what you mean. Are you proposing we introduce a value-based GFuture API on top of GTask?
A Future is in general a different concept, and doesn't always integrate well with the async paradigm...
Comment 8 Dan Winship 2013-01-02 18:04:59 UTC
(In reply to comment #3)
> Shall I attach an updated patch?

yes
Comment 9 Giovanni Campagna 2013-01-02 18:09:25 UTC
Created attachment 232546 [details] [review]
GTask: add g_task_wait_sync() for synchronizing with worker threads

This new API allows the mainloop to wait for a background task, in
case for whatever reason the result is needed now, rather than a
later time.
Comment 10 Dan Winship 2013-01-02 18:55:09 UTC
Comment on attachment 232546 [details] [review]
GTask: add g_task_wait_sync() for synchronizing with worker threads

> /**
>+ * g_task_wait_sync:
>+ * @task: a #GTask

I think it would be good to explicitly state that it does not internally run the main loop. How about:

  Synchronously wait for the completion of @task, which must have been been
  run with g_task_run_in_thread(). If @task's callback has already run, this
  function just returns immediately. Otherwise, it blocks until @task's
  #GTaskThreadFunc completes, and then calls @task's callback itself. (It does
  not iterate @task's #GMainContext.)

  You must call this function from the same thread that created @task.

  Since: 2.36

>+  while (!task->thread_complete && !task->idle_complete_source)

Do you actually need to check task->idle_complete_source here? I don't think it's possible for that to be set if task->thread_complete is FALSE.
Comment 11 Dan Winship 2013-01-02 18:55:33 UTC
oh, also, needs a test added to gio/tests/task.c
Comment 12 Giovanni Campagna 2013-02-22 14:04:02 UTC
In the end, the main reason for this API (background work in mutter) was done differently, so I'm no longer interested in it.
I'm closing the bug. If someone else likes it and wants to bring it forward, please reopen.