shaba/ostree
1
0
Fork 0
mirror of https://github.com/ostreedev/ostree.git synced 2025-03-16 10:50:43 +03:00
Code Issues Projects Releases Wiki Activity

fetcher: Fix another finalization deadlock

Browse Source 
If the current repo is already up to date (we have no content to
fetch), it's possible for the fetcher to not request any URIs.  So
create and then finalize it quickly.

Finalization involves calling `g_main_loop_quit()` +
`g_thread_wait()`.  However, if `g_main_loop_quit()` is run *before*
`g_main_loop_run()`, we'll deadlock because GMainLoop assumes in
`_run()` to start things.

This is a common trap - ideally, GMainLoop would record if `_quit()`
was called before `_run()` or something, but doing that now would
likely break people who are expecting quit() -> run() to restart.

In general, we've moved in various GLib-consuming apps to an
explicit "main context iteration with termination condition" model;
see `pull_termination_condition()` in the pull code.

This fixes this race condition.

I verified that an assertion in `_finalize` that more than
zero URIs were requested was hit in multiple test cases, and this patch
has survived a while of make check loops.

Closes: https://github.com/ostreedev/ostree/issues/496

Closes: #499
Approved by: jlebon
This commit is contained in:
Colin Walters 2016-09-08 12:39:42 -04:00 committed by Atomic Bot
parent 23b4b58d57
commit 82b756783b
1 changed files with 10 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/libostree/ostree-fetcher.c
View File

@ -46,7 +46,7 @@ typedef struct {
SoupSession *session; /* not referenced */
GMainContext *main_context;
GMainLoop *main_loop;
volatile gint running;
int tmpdir_dfd;
char *tmpdir_name;
@ -144,7 +144,6 @@ thread_closure_unref (ThreadClosure *thread_closure)
g_assert (thread_closure->session == NULL);
g_clear_pointer (&thread_closure->main_context, g_main_context_unref);
g_clear_pointer (&thread_closure->main_loop, g_main_loop_unref);
if (thread_closure->tmpdir_dfd != -1)
close (thread_closure->tmpdir_dfd);
@ -503,7 +502,12 @@ ostree_fetcher_session_thread (gpointer data)
}
closure->max_outstanding = 3 * max_conns;
g_main_loop_run (closure->main_loop);
/* This model ensures we don't hit a race using g_main_loop_quit();
* see also what pull_termination_condition() in ostree-repo-pull.c
* is doing.
*/
while (g_atomic_int_get (&closure->running))
g_main_context_iteration (closure->main_context, TRUE);
/* Since the ThreadClosure may be finalized from any thread we
* unreference all data related to the SoupSession ourself to ensure
@ -567,7 +571,8 @@ _ostree_fetcher_finalize (GObject *object)
OstreeFetcher *self = OSTREE_FETCHER (object);
/* Terminate the session thread. */
g_main_loop_quit (self->thread_closure->main_loop);
g_atomic_int_set (&self->thread_closure->running, 0);
g_main_context_wakeup (self->thread_closure->main_context);
if (self->session_thread)
{
/* We need to explicitly synchronize to clean up TLS */
@ -594,7 +599,7 @@ _ostree_fetcher_constructed (GObject *object)
self->thread_closure = g_slice_new0 (ThreadClosure);
self->thread_closure->ref_count = 1;
self->thread_closure->main_context = g_main_context_ref (main_context);
self->thread_closure->main_loop = g_main_loop_new (main_context, FALSE);
self->thread_closure->running = 1;
self->thread_closure->tmpdir_dfd = -1;
self->thread_closure->tmpdir_lock = empty_lockfile;