pull: Go back to using one main context

xdg-app was hanging for me with v2015.8, but worked with v2015.7.
I narrowed things down to the GMainLoop/context commit, in which
we started pushing a temporary main context for synchronous
requests internally.

That's never really going to work with libsoup - there needs
to be a single main context which works on the socket.  Furthermore,
clients couldn't get progress messages that way.

For *other* internal uses where we added APIs that talk to the remote
repo, we cleanly push a temporary main context.

(Note that I kind of snuck in a change here around the GError handling
 in pulls that isn't strictly related but came up in testing)

diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c
index bc2b676d0f0ae665045d1f50f84ce186bec7aa78..37e0f99d5fbdb451bcfb59da8f569760352d03f5 100644 (file)
--- a/src/libostree/ostree-fetcher.c
+++ b/src/libostree/ostree-fetcher.c
@@ -731,6 +731,10 @@ fetch_uri_sync_on_complete (GObject        *object,
   data->done = TRUE;
 }
 
+/* Synchronously request a URI - will iterate the thread-default main
+ * context for historical reasons.  If you don't want that, push a
+ * temporary one.
+ */
 gboolean
 _ostree_fetcher_request_uri_to_membuf (OstreeFetcher  *fetcher,
                                        SoupURI        *uri,
@@ -754,8 +758,7 @@ _ostree_fetcher_request_uri_to_membuf (OstreeFetcher  *fetcher,
   if (g_cancellable_set_error_if_cancelled (cancellable, error))
     return FALSE;
 
-  mainctx = g_main_context_new ();
-  g_main_context_push_thread_default (mainctx);
+  mainctx = g_main_context_ref_thread_default ();
 
   data.done = FALSE;
   data.error = error;
@@ -800,8 +803,6 @@ _ostree_fetcher_request_uri_to_membuf (OstreeFetcher  *fetcher,
   ret = TRUE;
   *out_contents = g_memory_output_stream_steal_as_bytes (buf);
  out:
-  if (mainctx)
-    g_main_context_pop_thread_default (mainctx);
   g_clear_object (&(data.result_stream));
   return ret;
 }

diff --git a/src/libostree/ostree-metalink.c b/src/libostree/ostree-metalink.c
index 5ca69e6980ff994ad5ab6e11d31f93cc285ce889..083ad16fb2dd33be533df5eeae16626290b1b03c 100644 (file)
--- a/src/libostree/ostree-metalink.c
+++ b/src/libostree/ostree-metalink.c
@@ -589,6 +589,11 @@ typedef struct
   GMainLoop             *loop;
 } FetchMetalinkSyncData;
 
+/*
+ * Note that for legacy reasons we iterate the caller's main context.
+ * If you don't want that (and you probably don't) push a temporary
+ * one.
+ */
 gboolean
 _ostree_metalink_request_sync (OstreeMetalink        *self,
                                SoupURI               **out_target_uri,
@@ -607,8 +612,7 @@ _ostree_metalink_request_sync (OstreeMetalink        *self,
   if (fetching_sync_uri != NULL)
     *fetching_sync_uri = _ostree_metalink_get_uri (self);
 
-  mainctx = g_main_context_new ();
-  g_main_context_push_thread_default (mainctx);
+  mainctx = g_main_context_ref_thread_default ();
 
   request.metalink = g_object_ref (self);
   request.urls = g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free);
@@ -633,8 +637,6 @@ _ostree_metalink_request_sync (OstreeMetalink        *self,
 
   ret = TRUE;
  out:
-  if (mainctx)
-    g_main_context_pop_thread_default (mainctx);
   g_clear_object (&request.metalink);
   g_clear_pointer (&request.urls, g_ptr_array_unref);
   g_clear_pointer (&request.parser, g_markup_parse_context_free);

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index e82340abdcd94943572231ed219ab3f5f636edb1..28198a405333c6ad1386fa1d541db0fce0cb7ccf 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -96,6 +96,7 @@ typedef struct {
   guint64       previous_bytes_sec;
   guint64       previous_total_downloaded;
 
+  GError       *cached_async_error;
   GError      **async_error;
   gboolean      caught_error;
 
@@ -1697,7 +1698,10 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   pull_data->is_mirror = (flags & OSTREE_REPO_PULL_FLAGS_MIRROR) > 0;
   pull_data->is_commit_only = (flags & OSTREE_REPO_PULL_FLAGS_COMMIT_ONLY) > 0;
 
-  pull_data->async_error = error;
+  if (error)
+    pull_data->async_error = &pull_data->cached_async_error;
+  else
+    pull_data->async_error = NULL;
   pull_data->main_context = g_main_context_ref_thread_default ();
   pull_data->flags = flags;
 
@@ -2227,6 +2231,15 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
 
   ret = TRUE;
  out:
+  /* This is pretty ugly - we have two error locations, because we
+   * have a mix of synchronous and async code.  Mixing them gets messy
+   * as we need to avoid overwriting errors.
+   */
+  if (pull_data->cached_async_error && error && !*error)
+    g_propagate_error (error, pull_data->cached_async_error);
+  else
+    g_clear_error (&pull_data->cached_async_error);
+    
   ostree_repo_abort_transaction (pull_data->repo, cancellable, NULL);
   g_main_context_unref (pull_data->main_context);
   if (update_timeout)

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index c1ce864edb8230af94a9714ba874a13391aa3fce..8d2fa93abce47c153cda69285eb7a9c641e61c07 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -1681,6 +1681,7 @@ _ostree_preload_metadata_file (OstreeRepo    *self,
                                GError        **error)
 {
   gboolean ret = FALSE;
+
   if (is_metalink)
     {
       glnx_unref_object OstreeMetalink *metalink = NULL;
@@ -1740,12 +1741,16 @@ repo_remote_fetch_summary (OstreeRepo    *self,
                            GError       **error)
 {
   glnx_unref_object OstreeFetcher *fetcher = NULL;
+  g_autoptr(GMainContext) mainctx = NULL;
   gboolean ret = FALSE;
   SoupURI *base_uri = NULL;
   uint i;
   const char *filenames[] = {"summary", "summary.sig"};
   GBytes **outputs[] = {out_summary, out_signatures};
 
+  mainctx = g_main_context_new ();
+  g_main_context_push_thread_default (mainctx);
+
   fetcher = _ostree_repo_remote_new_fetcher (self, name, error);
   if (fetcher == NULL)
     goto out;
@@ -1787,9 +1792,10 @@ repo_remote_fetch_summary (OstreeRepo    *self,
   ret = TRUE;
 
  out:
+  if (mainctx)
+    g_main_context_pop_thread_default (mainctx);
   if (base_uri != NULL)
     soup_uri_free (base_uri);
-
   return ret;
 }