GNOME Bugzilla – Bug 780299
Do not crash while leaving the panel during actualize_printers_list ()
Last modified: 2017-04-10 12:26:26 UTC
Filed downstream as https://bugzilla.redhat.com/show_bug.cgi?id=1432257
Created attachment 348312 [details] [review] printers: Make actualize_printers_list cancellable This way we can safely interrupt an update without crashing g-c-c.
Review of attachment 348312 [details] [review]: The printers panel still crashes with the patch. You can test it simply by inserting "g_usleep(3000000);" into "_pp_cups_get_dests_thread()". You should somehow check that the operation was cancelled. E.g. by "g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)" in the callback.
Created attachment 348392 [details] [review] printers: Make actualize_printers_list cancellable This way we can safely interrupt an update without crashing g-c-c.
Review of attachment 348392 [details] [review]: Printers panel is still crashing for me with the patch. ::: panels/printers/cc-printers-panel.c @@ +91,3 @@ GCancellable *get_all_ppds_cancellable; GCancellable *subscription_renew_cancellable; + GCancellable *atualize_printers_list_cancellable; You have a typo here. @@ +640,3 @@ priv = PRINTERS_PANEL_PRIVATE (self); free_dests (self); These usages of "self" should be after the check for an error. I'm surprised that it doesn't crash for you. @@ +643,3 @@ + cups_dests = pp_cups_get_dests_finish (cups, result, &error); + + if (error != NULL) You should check also "cups_dests == NULL" here since GTask will return NULL in case of an error. @@ +1028,2 @@ priv->subscription_renew_cancellable = g_cancellable_new (); + priv->atualize_printers_list_cancellable = g_cancellable_new (); Initialize this sooner otherwise the handler of a permission change will use NULL cancellable which is another cause of crash which I can reproduce here.
Created attachment 348644 [details] [review] printers: Make actualize_printers_list cancellable This way we can safely interrupt an update without crashing g-c-c.
Review of attachment 348644 [details] [review]: ::: panels/printers/cc-printers-panel.c @@ +638,3 @@ int i; priv = PRINTERS_PANEL_PRIVATE (self); I meant also this usage of "self" to move below the check for error. @@ +644,1 @@ + if (cups_dests == NULL || error != NULL) Change the "||" to "&&" please.
Created attachment 348804 [details] [review] printers: Make actualize_printers_list cancellable This way we can safely interrupt an update without crashing g-c-c.
Review of attachment 348804 [details] [review]: You've posted the same patch as before.
Created attachment 348806 [details] [review] printers: Make actualize_printers_list cancellable This way we can safely interrupt an update without crashing g-c-c.
(In reply to Marek Kašík from comment #8) > Review of attachment 348804 [details] [review] [review]: > > You've posted the same patch as before. heh, sorry. did not git add the changes.
Review of attachment 348806 [details] [review]: Thank you for the patch. Please push it to master and gnome-3-24 branches. Since the patch is not applicable to 3.22, prepare a version for gnome-3-22 branch and attach it here please. I'll review it.
Comment on attachment 348806 [details] [review] printers: Make actualize_printers_list cancellable Thanks again Marek! Attachment 348806 [details] pushed as 519e19f - printers: Make actualize_printers_list cancellable
Created attachment 349353 [details] [review] printers: Make actualize_printers_list* cancellable This way we can safely interrupt an update without crashing g-c-c.
(In reply to Felipe Borges from comment #13) > Created attachment 349353 [details] [review] [review] > printers: Make actualize_printers_list* cancellable this is the backported version of the patch for gnome-3-22.
Review of attachment 349353 [details] [review]: Thank you for the patch. Push it to gnome-3-22 branch please.
Thanks! Attachment 349353 [details] pushed as b8b0347 - printers: Make actualize_printers_list* cancellable