From 74936f98d89514820c4d1ec574269fd0a140f1da Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 28 Oct 2019 14:04:55 -0400 Subject: [PATCH] lib/pull: Tweak update_timeout logic again I was hitting `SIGSEGV` when running `cosa build` and narrowed it down to #1954. What's happening here is that because we're using the default context, when we unref it in the out path, it may not actually destroy the `GSource` if it (the context) is still ref'ed elsewhere. So then, we'd still get events from it if subsequent operations iterated the context. This patch is mostly a revert of #1954, except that we still keep a ref on the `GSource`. That way it is always safe to destroy it afterwards. (And I've also added a comment to explain this better.) --- src/libostree/ostree-repo-pull.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index dd4dbff1..586b34fe 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -3573,6 +3573,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_autofree char **refs_to_fetch = NULL; g_autoptr(GVariantIter) collection_refs_iter = NULL; g_autofree char **override_commit_ids = NULL; + g_autoptr(GSource) update_timeout = NULL; gboolean opt_gpg_verify_set = FALSE; gboolean opt_gpg_verify_summary_set = FALSE; gboolean opt_collection_refs_set = FALSE; @@ -3674,6 +3675,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, pull_data->async_error = &pull_data->cached_async_error; else pull_data->async_error = NULL; + + /* Note we're using the thread default (or global) context here, so it may outlive the + * OtPullData object if there's another ref on it. Thus, always detach/destroy sources + * local to the `ostree_repo_pull*` operation rather than trying to transfer ownership. */ pull_data->main_context = g_main_context_ref_thread_default (); pull_data->flags = flags; @@ -4504,8 +4509,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (pull_data->progress) { - g_autoptr(GSource) update_timeout = NULL; - /* Setup a custom frequency if set */ if (update_frequency > 0) update_timeout = g_timeout_source_new (pull_data->dry_run ? 0 : update_frequency); @@ -4732,6 +4735,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!inherit_transaction) ostree_repo_abort_transaction (pull_data->repo, cancellable, NULL); g_main_context_unref (pull_data->main_context); + if (update_timeout) + g_source_destroy (update_timeout); g_strfreev (configured_branches); g_clear_object (&pull_data->fetcher); g_clear_pointer (&pull_data->extra_headers, (GDestroyNotify)g_variant_unref);