[gnome-software: 4/7] gs-metered: Remove Mogwai schedule entries when downloads are complete
- From: Phaedrus Leeds <mwleeds src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software: 4/7] gs-metered: Remove Mogwai schedule entries when downloads are complete
- Date: Thu, 21 Jan 2021 02:16:04 +0000 (UTC)
commit 3e635df34da24704b8293fc12cc3bb0a1a0c12e7
Author: Philip Withnall <withnall endlessm com>
Date: Fri May 22 13:24:26 2020 +0100
gs-metered: Remove Mogwai schedule entries when downloads are complete
This was an oversight when the metered data support was originally
added: the schedule entry for a download is not automatically removed
when the last reference to the `MwscScheduleEntry` is dropped, and it
has to be removed manually.
Add a new function to do that, and call it explicitly when the download
is complete (or has failed). A previous version of this patch tried to
tie it to the `GsApp:state` or `GsAppList:state` of the download, but
that was too fragile.
This does break the API in `gs-metered.h`. There’s no real way around
that, other than by deprecating the existing function and adding a
replacement. Since metered data is not widely used yet, I thought the
need for that was low.
Signed-off-by: Philip Withnall <withnall endlessm com>
lib/gs-metered.c | 74 +++++++++++++++++++++--
lib/gs-metered.h | 6 ++
plugins/flatpak/gs-plugin-flatpak.c | 69 +++++++++++++--------
plugins/fwupd/gs-plugin-fwupd.c | 32 ++++++----
plugins/packagekit/gs-plugin-packagekit-refresh.c | 12 +++-
5 files changed, 149 insertions(+), 44 deletions(-)
---
diff --git a/lib/gs-metered.c b/lib/gs-metered.c
index f4c8aff8..17c42a08 100644
--- a/lib/gs-metered.c
+++ b/lib/gs-metered.c
@@ -82,6 +82,8 @@ invalidated_cb (MwscScheduleEntry *entry,
* gs_metered_block_on_download_scheduler:
* @parameters: (nullable): a #GVariant of type `a{sv}` specifying parameters
* for the schedule entry, or %NULL to pass no parameters
+ * @schedule_entry_handle_out: (out) (not optional): return location for a
+ * handle to the resulting schedule entry
* @cancellable: (nullable): a #GCancellable, or %NULL
* @error: return location for a #GError, or %NULL
*
@@ -90,6 +92,10 @@ invalidated_cb (MwscScheduleEntry *entry,
*
* FIXME: This will currently ignore later revocations of that download
* permission, and does not support creating a schedule entry per app.
+ * The schedule entry must later be removed from the schedule by passing
+ * the handle from @schedule_entry_handle_out to
+ * gs_metered_remove_from_download_scheduler(), otherwise resources will leak.
+ * This is an opaque handle and should not be inspected.
*
* If a schedule entry cannot be created, or if @cancellable is cancelled,
* an error will be set and %FALSE returned.
@@ -100,10 +106,11 @@ invalidated_cb (MwscScheduleEntry *entry,
* This function will likely be called from a #GsPluginLoader worker thread.
*
* Returns: %TRUE on success, %FALSE otherwise
- * Since: 3.34
+ * Since: 3.38
*/
gboolean
gs_metered_block_on_download_scheduler (GVariant *parameters,
+ gpointer *schedule_entry_handle_out,
GCancellable *cancellable,
GError **error)
{
@@ -114,6 +121,11 @@ gs_metered_block_on_download_scheduler (GVariant *parameters,
g_autoptr(GMainContext) context = NULL;
g_autoptr(GsMainContextPusher) pusher = NULL;
+ g_return_val_if_fail (schedule_entry_handle_out != NULL, FALSE);
+
+ /* set this in case of error */
+ *schedule_entry_handle_out = NULL;
+
parameters_str = (parameters != NULL) ? g_variant_print (parameters, TRUE) : g_strdup ("(none)");
g_debug ("%s: Waiting with parameters: %s", G_STRFUNC, parameters_str);
@@ -160,15 +172,21 @@ gs_metered_block_on_download_scheduler (GVariant *parameters,
g_signal_handler_disconnect (schedule_entry, notify_id);
if (!download_now && invalidated_error != NULL) {
+ /* no need to remove the schedule entry as it’s been
+ * invalidated */
g_propagate_error (error, g_steal_pointer (&invalidated_error));
return FALSE;
} else if (!download_now && g_cancellable_set_error_if_cancelled (cancellable, error)) {
+ /* remove the schedule entry and fail */
+ gs_metered_remove_from_download_scheduler (schedule_entry, NULL, NULL);
return FALSE;
}
g_assert (download_now);
}
+ *schedule_entry_handle_out = g_object_ref (schedule_entry);
+
g_debug ("%s: Allowed to download", G_STRFUNC);
#else /* if !HAVE_MOGWAI */
g_debug ("%s: Allowed to download (Mogwai support compiled out)", G_STRFUNC);
@@ -177,9 +195,51 @@ gs_metered_block_on_download_scheduler (GVariant *parameters,
return TRUE;
}
+/**
+ * gs_metered_remove_from_download_scheduler:
+ * @schedule_entry_handle: (transfer full) (nullable): schedule entry handle as
+ * returned by gs_metered_block_on_download_scheduler()
+ * @cancellable: (nullable): a #GCancellable, or %NULL
+ * @error: return location for a #GError, or %NULL
+ *
+ * Remove a schedule entry previously created by
+ * gs_metered_block_on_download_scheduler(). This must be called after
+ * gs_metered_block_on_download_scheduler() has successfully returned, or
+ * resources will leak. It should be called once the corresponding download is
+ * complete.
+ *
+ * Returns: %TRUE on success, %FALSE otherwise
+ * Since: 3.38
+ */
+gboolean
+gs_metered_remove_from_download_scheduler (gpointer schedule_entry_handle,
+ GCancellable *cancellable,
+ GError **error)
+{
+#ifdef HAVE_MOGWAI
+ g_autoptr(MwscScheduleEntry) schedule_entry = schedule_entry_handle;
+#endif
+
+ g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE);
+ g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
+
+ g_debug ("Removing schedule entry handle %p", schedule_entry_handle);
+
+ if (schedule_entry_handle == NULL)
+ return TRUE;
+
+#ifdef HAVE_MOGWAI
+ return mwsc_schedule_entry_remove (schedule_entry, cancellable, error);
+#else
+ return TRUE;
+#endif
+}
+
/**
* gs_metered_block_app_on_download_scheduler:
* @app: a #GsApp to get the scheduler parameters from
+ * @schedule_entry_handle_out: (out) (not optional): return location for a
+ * handle to the resulting schedule entry
* @cancellable: a #GCancellable, or %NULL
* @error: return location for a #GError, or %NULL
*
@@ -187,10 +247,11 @@ gs_metered_block_on_download_scheduler (GVariant *parameters,
* download parameters from the given @app.
*
* Returns: %TRUE on success, %FALSE otherwise
- * Since: 3.34
+ * Since: 3.38
*/
gboolean
gs_metered_block_app_on_download_scheduler (GsApp *app,
+ gpointer *schedule_entry_handle_out,
GCancellable *cancellable,
GError **error)
{
@@ -209,12 +270,14 @@ gs_metered_block_app_on_download_scheduler (GsApp *app,
parameters = g_variant_ref_sink (g_variant_dict_end (¶meters_dict));
- return gs_metered_block_on_download_scheduler (parameters, cancellable, error);
+ return gs_metered_block_on_download_scheduler (parameters, schedule_entry_handle_out, cancellable,
error);
}
/**
* gs_metered_block_app_list_on_download_scheduler:
* @app_list: a #GsAppList to get the scheduler parameters from
+ * @schedule_entry_handle_out: (out) (not optional): return location for a
+ * handle to the resulting schedule entry
* @cancellable: a #GCancellable, or %NULL
* @error: return location for a #GError, or %NULL
*
@@ -222,10 +285,11 @@ gs_metered_block_app_on_download_scheduler (GsApp *app,
* download parameters from the apps in the given @app_list.
*
* Returns: %TRUE on success, %FALSE otherwise
- * Since: 3.34
+ * Since: 3.38
*/
gboolean
gs_metered_block_app_list_on_download_scheduler (GsAppList *app_list,
+ gpointer *schedule_entry_handle_out,
GCancellable *cancellable,
GError **error)
{
@@ -244,5 +308,5 @@ gs_metered_block_app_list_on_download_scheduler (GsAppList *app_list,
* prioritisation, so go with this simple implementation for now. */
parameters = g_variant_ref_sink (g_variant_dict_end (¶meters_dict));
- return gs_metered_block_on_download_scheduler (parameters, cancellable, error);
+ return gs_metered_block_on_download_scheduler (parameters, schedule_entry_handle_out, cancellable,
error);
}
diff --git a/lib/gs-metered.h b/lib/gs-metered.h
index f8cdc1fe..f81d1711 100644
--- a/lib/gs-metered.h
+++ b/lib/gs-metered.h
@@ -17,12 +17,18 @@
G_BEGIN_DECLS
gboolean gs_metered_block_on_download_scheduler (GVariant *parameters,
+ gpointer *schedule_entry_handle_out,
GCancellable *cancellable,
GError **error);
+gboolean gs_metered_remove_from_download_scheduler (gpointer schedule_entry_handle,
+ GCancellable *cancellable,
+ GError **error);
gboolean gs_metered_block_app_on_download_scheduler (GsApp *app,
+ gpointer *schedule_entry_handle_out,
GCancellable *cancellable,
GError **error);
gboolean gs_metered_block_app_list_on_download_scheduler (GsAppList *app_list,
+ gpointer *schedule_entry_handle_out,
GCancellable *cancellable,
GError **error);
diff --git a/plugins/flatpak/gs-plugin-flatpak.c b/plugins/flatpak/gs-plugin-flatpak.c
index 9c84f84b..d81739cb 100644
--- a/plugins/flatpak/gs-plugin-flatpak.c
+++ b/plugins/flatpak/gs-plugin-flatpak.c
@@ -633,6 +633,15 @@ _build_transaction (GsPlugin *plugin, GsFlatpak *flatpak,
return g_steal_pointer (&transaction);
}
+static void
+remove_schedule_entry (gpointer schedule_entry_handle)
+{
+ g_autoptr(GError) error_local = NULL;
+
+ if (!gs_metered_remove_from_download_scheduler (schedule_entry_handle, NULL, &error_local))
+ g_warning ("Failed to remove schedule entry: %s", error_local->message);
+}
+
gboolean
gs_plugin_download (GsPlugin *plugin, GsAppList *list,
GCancellable *cancellable, GError **error)
@@ -648,21 +657,12 @@ gs_plugin_download (GsPlugin *plugin, GsAppList *list,
GsFlatpak *flatpak = GS_FLATPAK (key);
GsAppList *list_tmp = GS_APP_LIST (value);
g_autoptr(FlatpakTransaction) transaction = NULL;
+ gpointer schedule_entry_handle = NULL;
g_assert (GS_IS_FLATPAK (flatpak));
g_assert (list_tmp != NULL);
g_assert (gs_app_list_length (list_tmp) > 0);
- if (!gs_plugin_has_flags (plugin, GS_PLUGIN_FLAGS_INTERACTIVE)) {
- g_autoptr(GError) error_local = NULL;
-
- if (!gs_metered_block_app_list_on_download_scheduler (list_tmp, cancellable,
&error_local)) {
- g_warning ("Failed to block on download scheduler: %s",
- error_local->message);
- g_clear_error (&error_local);
- }
- }
-
/* build and run non-deployed transaction */
transaction = _build_transaction (plugin, flatpak, cancellable, error);
if (transaction == NULL) {
@@ -701,11 +701,25 @@ gs_plugin_download (GsPlugin *plugin, GsAppList *list,
return FALSE;
}
}
+
+ if (!gs_plugin_has_flags (plugin, GS_PLUGIN_FLAGS_INTERACTIVE)) {
+ g_autoptr(GError) error_local = NULL;
+
+ if (!gs_metered_block_app_list_on_download_scheduler (list_tmp,
&schedule_entry_handle, cancellable, &error_local)) {
+ g_warning ("Failed to block on download scheduler: %s",
+ error_local->message);
+ g_clear_error (&error_local);
+ }
+ }
+
if (!gs_flatpak_transaction_run (transaction, cancellable, error)) {
gs_flatpak_error_convert (error);
+ remove_schedule_entry (schedule_entry_handle);
return FALSE;
}
+ remove_schedule_entry (schedule_entry_handle);
+
/* Traverse over the GsAppList again and set that the update has been already downloaded
* for the apps. */
for (guint i = 0; i < gs_app_list_length (list_tmp); i++) {
@@ -800,6 +814,8 @@ gs_plugin_app_install (GsPlugin *plugin,
GsPluginData *priv = gs_plugin_get_data (plugin);
GsFlatpak *flatpak;
g_autoptr(FlatpakTransaction) transaction = NULL;
+ g_autoptr(GError) error_local = NULL;
+ gpointer schedule_entry_handle = NULL;
/* queue for install if installation needs the network */
if (!app_has_local_source (app) &&
@@ -834,19 +850,6 @@ gs_plugin_app_install (GsPlugin *plugin,
if (gs_app_get_kind (app) == AS_APP_KIND_SOURCE)
return gs_flatpak_app_install_source (flatpak, app, cancellable, error);
- if (!gs_plugin_has_flags (plugin, GS_PLUGIN_FLAGS_INTERACTIVE)) {
- g_autoptr(GError) error_local = NULL;
-
- /* FIXME: Add additional details here, especially the download
- * size bounds (using `size-minimum` and `size-maximum`, both
- * type `t`). */
- if (!gs_metered_block_app_on_download_scheduler (app, cancellable, &error_local)) {
- g_warning ("Failed to block on download scheduler: %s",
- error_local->message);
- g_clear_error (&error_local);
- }
- }
-
/* build */
transaction = _build_transaction (plugin, flatpak, cancellable, error);
if (transaction == NULL) {
@@ -908,14 +911,28 @@ gs_plugin_app_install (GsPlugin *plugin,
}
}
+ if (!gs_plugin_has_flags (plugin, GS_PLUGIN_FLAGS_INTERACTIVE)) {
+ /* FIXME: Add additional details here, especially the download
+ * size bounds (using `size-minimum` and `size-maximum`, both
+ * type `t`). */
+ if (!gs_metered_block_app_on_download_scheduler (app, &schedule_entry_handle, cancellable,
&error_local)) {
+ g_warning ("Failed to block on download scheduler: %s",
+ error_local->message);
+ g_clear_error (&error_local);
+ }
+ }
+
/* run transaction */
gs_app_set_state (app, GS_APP_STATE_INSTALLING);
if (!gs_flatpak_transaction_run (transaction, cancellable, error)) {
gs_flatpak_error_convert (error);
gs_app_set_state_recover (app);
+ remove_schedule_entry (schedule_entry_handle);
return FALSE;
}
+ remove_schedule_entry (schedule_entry_handle);
+
/* get any new state */
if (!gs_flatpak_refresh (flatpak, G_MAXUINT, cancellable, error)) {
gs_flatpak_error_convert (error);
@@ -941,6 +958,7 @@ gs_plugin_flatpak_update (GsPlugin *plugin,
{
g_autoptr(FlatpakTransaction) transaction = NULL;
gboolean is_update_downloaded = TRUE;
+ gpointer schedule_entry_handle = NULL;
/* build and run transaction */
transaction = _build_transaction (plugin, flatpak, cancellable, error);
@@ -998,7 +1016,7 @@ gs_plugin_flatpak_update (GsPlugin *plugin,
} else if (!gs_plugin_has_flags (plugin, GS_PLUGIN_FLAGS_INTERACTIVE)) {
g_autoptr(GError) error_local = NULL;
- if (!gs_metered_block_app_list_on_download_scheduler (list_tmp, cancellable, &error_local)) {
+ if (!gs_metered_block_app_list_on_download_scheduler (list_tmp, &schedule_entry_handle,
cancellable, &error_local)) {
g_warning ("Failed to block on download scheduler: %s",
error_local->message);
g_clear_error (&error_local);
@@ -1016,8 +1034,11 @@ gs_plugin_flatpak_update (GsPlugin *plugin,
gs_app_set_state_recover (app);
}
gs_flatpak_error_convert (error);
+ remove_schedule_entry (schedule_entry_handle);
return FALSE;
}
+
+ remove_schedule_entry (schedule_entry_handle);
gs_plugin_updates_changed (plugin);
/* get any new state */
diff --git a/plugins/fwupd/gs-plugin-fwupd.c b/plugins/fwupd/gs-plugin-fwupd.c
index 3b2fc387..23e47d58 100644
--- a/plugins/fwupd/gs-plugin-fwupd.c
+++ b/plugins/fwupd/gs-plugin-fwupd.c
@@ -995,6 +995,8 @@ gs_plugin_download_app (GsPlugin *plugin,
#endif
GFile *local_file;
g_autofree gchar *filename = NULL;
+ gpointer schedule_entry_handle = NULL;
+ g_autoptr(GError) error_local = NULL;
/* only process this app if was created by this plugin */
if (g_strcmp0 (gs_app_get_management_plugin (app),
@@ -1019,11 +1021,11 @@ gs_plugin_download_app (GsPlugin *plugin,
#if FWUPD_CHECK_VERSION(1,5,2)
g_autoptr(GFile) file = g_file_new_for_path (filename);
#endif
+ gboolean download_success;
+ g_autoptr(GError) error_local = NULL;
if (!gs_plugin_has_flags (plugin, GS_PLUGIN_FLAGS_INTERACTIVE)) {
- g_autoptr(GError) error_local = NULL;
-
- if (!gs_metered_block_app_on_download_scheduler (app, cancellable, &error_local)) {
+ if (!gs_metered_block_app_on_download_scheduler (app, &schedule_entry_handle,
cancellable, &error_local)) {
g_warning ("Failed to block on download scheduler: %s",
error_local->message);
g_clear_error (&error_local);
@@ -1031,19 +1033,23 @@ gs_plugin_download_app (GsPlugin *plugin,
}
#if FWUPD_CHECK_VERSION(1,5,2)
- if (!fwupd_client_download_file (priv->client,
- uri, file,
- FWUPD_CLIENT_DOWNLOAD_FLAG_NONE,
- cancellable,
- error)) {
+ download_success = fwupd_client_download_file (priv->client,
+ uri, file,
+ FWUPD_CLIENT_DOWNLOAD_FLAG_NONE,
+ cancellable,
+ error);
+ if (!download_success)
gs_plugin_fwupd_error_convert (error);
- return FALSE;
- }
#else
- if (!gs_plugin_download_file (plugin, app, uri, filename,
- cancellable, error))
- return FALSE;
+ download_success = gs_plugin_download_file (plugin, app, uri, filename,
+ cancellable, error);
#endif
+
+ if (!gs_metered_remove_from_download_scheduler (schedule_entry_handle, NULL, &error_local))
+ g_warning ("Failed to remove schedule entry: %s", error_local->message);
+
+ if (!download_success)
+ return FALSE;
}
gs_app_set_size_download (app, 0);
return TRUE;
diff --git a/plugins/packagekit/gs-plugin-packagekit-refresh.c
b/plugins/packagekit/gs-plugin-packagekit-refresh.c
index 2d9a7e46..1f661791 100644
--- a/plugins/packagekit/gs-plugin-packagekit-refresh.c
+++ b/plugins/packagekit/gs-plugin-packagekit-refresh.c
@@ -112,6 +112,9 @@ gs_plugin_download (GsPlugin *plugin,
GError **error)
{
g_autoptr(GsAppList) list_tmp = gs_app_list_new ();
+ g_autoptr(GError) error_local = NULL;
+ gboolean retval;
+ gpointer schedule_entry_handle = NULL;
/* add any packages */
for (guint i = 0; i < gs_app_list_length (list); i++) {
@@ -139,14 +142,19 @@ gs_plugin_download (GsPlugin *plugin,
if (!gs_plugin_has_flags (plugin, GS_PLUGIN_FLAGS_INTERACTIVE)) {
g_autoptr(GError) error_local = NULL;
- if (!gs_metered_block_app_list_on_download_scheduler (list_tmp, cancellable, &error_local)) {
+ if (!gs_metered_block_app_list_on_download_scheduler (list_tmp, &schedule_entry_handle,
cancellable, &error_local)) {
g_warning ("Failed to block on download scheduler: %s",
error_local->message);
g_clear_error (&error_local);
}
}
- return _download_only (plugin, list_tmp, cancellable, error);
+ retval = _download_only (plugin, list_tmp, cancellable, error);
+
+ if (!gs_metered_remove_from_download_scheduler (schedule_entry_handle, NULL, &error_local))
+ g_warning ("Failed to remove schedule entry: %s", error_local->message);
+
+ return retval;
}
gboolean
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
