Looks like this v2 never made to the list? I am sending it again.
On 03/02/17 15:28, Eduardo Lima (Etrunko) wrote:
> We must take into account that users can close the dialog at anytime,
> even during an operation of fetch or set ISO has not been finished. This
> will cause the callbacks for those operations to be invoked with an
> invalid object, crashing the application.
>
> To fix this issue we need to pass a GCancellable to the asynchronous
> operations, so they can be cancelled if the dialog happens to get closed
> before they complete.
>
> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@xxxxxxxxxx>
> ---
> v2:
> * Move call to g_cancellable_cancel() to response handler.
> * Don't leak error objects if operation is cancelled.
>
> ---
> src/remote-viewer-iso-list-dialog.c | 42 ++++++++++++++++++++++++++++++-------
> 1 file changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c
> index f23ddb2..aa0ebbb 100644
> --- a/src/remote-viewer-iso-list-dialog.c
> +++ b/src/remote-viewer-iso-list-dialog.c
> @@ -42,6 +42,7 @@ struct _RemoteViewerISOListDialogPrivate
> GtkWidget *stack;
> GtkWidget *tree_view;
> OvirtForeignMenu *foreign_menu;
> + GCancellable *cancellable;
> };
>
> enum RemoteViewerISOListDialogModel
> @@ -66,6 +67,8 @@ remote_viewer_iso_list_dialog_dispose(GObject *object)
> RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
> RemoteViewerISOListDialogPrivate *priv = self->priv;
>
> + g_clear_object(&priv->cancellable);
> +
> if (priv->foreign_menu) {
> g_signal_handlers_disconnect_by_data(priv->foreign_menu, object);
> g_clear_object(&priv->foreign_menu);
> @@ -157,17 +160,24 @@ fetch_iso_names_cb(OvirtForeignMenu *foreign_menu,
> const gchar *msg = error ? error->message : _("Failed to fetch CD names");
> gchar *markup = g_strdup_printf("<b>%s</b>", msg);
>
> + g_debug("Error fetching ISO names: %s", msg);
> + if (error && error->code == G_IO_ERROR_CANCELLED)
> + goto end;
> +
> gtk_label_set_markup(GTK_LABEL(priv->status), markup);
> gtk_spinner_stop(GTK_SPINNER(priv->spinner));
> remote_viewer_iso_list_dialog_show_error(self, msg);
> gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE);
> g_free(markup);
> - g_clear_error(&error);
> - return;
> + goto end;
> }
>
> + g_clear_object(&priv->cancellable);
> g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, self);
> remote_viewer_iso_list_dialog_show_files(self);
> +
> +end:
> + g_clear_error(&error);
> }
>
>
> @@ -177,7 +187,10 @@ remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self)
> RemoteViewerISOListDialogPrivate *priv = self->priv;
>
> gtk_list_store_clear(priv->list_store);
> - ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, NULL,
> +
> + priv->cancellable = g_cancellable_new();
> + ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu,
> + priv->cancellable,
> (GAsyncReadyCallback) fetch_iso_names_cb,
> self);
> }
> @@ -190,8 +203,10 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
> RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
> RemoteViewerISOListDialogPrivate *priv = self->priv;
>
> - if (response_id != GTK_RESPONSE_NONE)
> + if (response_id != GTK_RESPONSE_NONE) {
> + g_cancellable_cancel(priv->cancellable);
> return;
> + }
>
> gtk_spinner_start(GTK_SPINNER(priv->spinner));
> gtk_label_set_markup(GTK_LABEL(priv->status), _("<b>Loading...</b>"));
> @@ -223,7 +238,9 @@ remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer G_GNU
> gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE);
> gtk_widget_set_sensitive(priv->tree_view, FALSE);
>
> - ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name, NULL,
> + priv->cancellable = g_cancellable_new();
> + ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name,
> + priv->cancellable,
> (GAsyncReadyCallback)ovirt_foreign_menu_iso_name_changed,
> self);
> gtk_tree_path_free(tree_path);
> @@ -308,12 +325,18 @@ ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu *foreign_menu,
> * change the ISO back to the original, avoiding a possible inconsistency.
> */
> if (!ovirt_foreign_menu_set_current_iso_name_finish(foreign_menu, result, &error)) {
> - remote_viewer_iso_list_dialog_show_error(self, error ? error->message : _("Failed to change CD"));
> - g_clear_error(&error);
> + gchar *msg = error ? error->message : _("Failed to change CD");
> + g_debug("Error changing ISO: %s", msg);
> +
> + if (error && error->code == G_IO_ERROR_CANCELLED)
> + goto end;
> +
> + remote_viewer_iso_list_dialog_show_error(self, msg);
> }
>
> + g_clear_object(&priv->cancellable);
> if (!gtk_tree_model_get_iter_first(model, &iter))
> - return;
> + goto end;
>
> current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu);
>
> @@ -340,6 +363,9 @@ ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu *foreign_menu,
> gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE);
> gtk_widget_set_sensitive(priv->tree_view, TRUE);
> g_free(current_iso);
> +
> +end:
> + g_clear_error(&error);
> }
>
> GtkWidget *
>
--
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko@xxxxxxxxxx
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
