GNOME Bugzilla – Bug 779846
Printers panel crashes in callback of details/options dialog
Last modified: 2017-03-13 11:49:06 UTC
If there is a change which needs actualization of printers list during showing of Options or Details dialog, the panel crashes due to already released PpPrinterEntry which opened it. To reproduce: 1) open a details dialog of a printer 2) run cupsdisable a-printer-name 3) run cupsenable a-printer-name 4) close the dialog The same happens when you add or remove a printer during the showing of the dialog.
Created attachment 347652 [details] [review] printers: Allow reseting of PpPrinterEntry children By introducing pp_printer_entry_setup () we allow PpPrinterEntry to be reset multiple times, allowing us to apply printer changes without having to destroy the PpPrinterEntry widgets.
Created attachment 347653 [details] [review] printers: Update printer entries individually Instead of actualizing the whole printers list on cups' PrinterAdded, PrinterDeleted event, reset the changed PrinterEntry individually.
I consider this to be a corner case, assuming it is not expected that a driver would touch the car engine while driving. :-) The proposed solution covers this case, BUT I don't consider it to be optimal in terms of code legibility. I would rather transform the PpPrinterEntry widgets which correspond to printer information to real GObject properties, so we could directly bind changes to a specific widget (for instance, changing ONLY a PpPrinterEntry->location_label when it changes). This is definitely feasible but I would rather not do such big structural changes so close to the release/code freeze.
I still get the crash with the patches. I think that we should focus on making the dialogs (options dialog, details dialog and jobs dialog) immune against releasing of the underlying printer entry. My proposal is to change callbacks of the dialogs once we are destroying the PpPrinterEntry. For this you will need 2 simple functions pp_jobs_dialog_set_callback() and pp_options_dialog_set_callback. For the details dialog it is enough to change it using g_signal_handlers_disconnect_by_data() and g_signal_connect(). The new callbacks would just free the structures given in user_data.
Created attachment 347806 [details] [review] printers: Make the dialogs independent of the PpPrinterEntry lifetime
Review of attachment 347806 [details] [review]: Thanks. Push it to master branch please.
Attachment 347806 [details] pushed as 909eb2a - printers: Make the dialogs independent of the PpPrinterEntry lifetime
(In reply to Marek Kašík from comment #6) > Review of attachment 347806 [details] [review] [review]: > > Thanks. Push it to master branch please. Should we chery-pick it to gnome-3-24 as well?
(In reply to Felipe Borges from comment #8) > (In reply to Marek Kašík from comment #6) > > Review of attachment 347806 [details] [review] [review] [review]: > > > > Thanks. Push it to master branch please. > > Should we chery-pick it to gnome-3-24 as well? Yes please, I didn't know that there is already gnome-3-24.
Attachment 347806 [details] pushed as 58270a2 (gnome-3-24) - printers: Make the dialogs independent of the PpPrinterEntry lifetime