GNOME Bugzilla – Bug 739003
Crashes in file_save_dialog_response()
Last modified: 2015-07-17 15:13:15 UTC
This bug was filed against the Fedora packages here: https://bugzilla.redhat.com/show_bug.cgi?id=1105795 It was originally reported against 2.8.10 but seems to occur on 2.8.14 as well. It was reported 47 times against 2.8.10 and 7 times against 2.8.14 which makes me believe that it's not that easily reproducible. See our retrace server report: https://retrace.fedoraproject.org/faf/reports/296586/ ... and the backtrace (against 2.8.10, but app/dialogs/file-save-dialog.c hasn't changed between that and 2.8.14): https://bugzilla.redhat.com/attachment.cgi?id=903167 That our users didn't describe what they did prior doesn't matter much -- judging from the trace, they tried to export or save an image file. --- 8< --- Program terminated with signal SIGSEGV, Segmentation fault.
+ Trace 234248
Thread 1 (Thread 0x7f7654f339c0 (LWP 10145))
Here's the relevant code around where it crashes: --- 8< --- app/dialogs/file-save-dialog.c --- 154 static void 155 file_save_dialog_response (GtkWidget *save_dialog, 156 gint response_id, 157 Gimp *gimp) 158 { 159 GimpFileDialog *dialog = GIMP_FILE_DIALOG (save_dialog); [...] 186 handler_id = g_signal_connect (dialog, "destroy", 187 G_CALLBACK (gtk_widget_destroyed), 188 &dialog); 189 190 switch (file_save_dialog_check_uri (save_dialog, gimp, 191 &uri, &basename, &save_proc)) 192 { 193 case CHECK_URI_FAIL: 194 break; 195 196 case CHECK_URI_OK: 197 gimp_file_dialog_set_sensitive (dialog, FALSE); 198 199 if (file_save_dialog_save_image (GIMP_PROGRESS (save_dialog), 200 gimp, 201 dialog->image, 202 uri, 203 save_proc, 204 GIMP_RUN_INTERACTIVE, 205 ! dialog->save_a_copy && ! dialog->export, 206 FALSE, 207 dialog->export, 208 FALSE)) 209 { 210 /* Save was successful, now store the URI in a couple of 211 * places that depend on it being the user that made a 212 * save. Lower-level URI management is handled in 213 * file_save() 214 */ 215 if (dialog->save_a_copy) 216 gimp_image_set_save_a_copy_uri (dialog->image, uri); --- >8 -------------------------------------- The crash happens in line 215, where dialog is dereferenced, but according to the backtrace is NULL. Immediately previous to that there's the call to file_save_dialog_save_imagefile_save_dialog_save_image() and while determining the arguments, dereferencing dialog still worked fine. I have a hunch that this is what happens: - During file_save_dialog_save_image(), save_dialog typecast as a GimpProgress object is destroyed. Possibly by way of gimp_plug_in_progress_end() -> gimp_free_progress() -- I'm unsure about that one. - The "destroy" signal for save_dialog (which is just differently typed name for dialog) is emitted. - gtk_widget_destroy(dialog, &dialog) is called which was installed as a signal handler in line 186. - dialog is NULL Does that sound plausible? I quickly skimmed over the code of file_save_dialog_response() in master and it seems to only have changed superficially since 2.8.10/2.8.14. If I don't hear objections, I'll write a patch which just checks for dialog != NULL at the beginning of the if (...) {} body. I'd rather not mask a different problem with it, however ;).
Maybe the proper solution is to g_object_ref the dialog at the beginning of the function, and g_object_unref the object at the end of it?
Yes the proper solution is to ref, given something is really destroying the dialog, but that should happen, so I'd prefer to figure the reason first. Calling end() on a GimpFileDialog (which implements the GimpProgress interface) definitely doesn't destroy the dialog, it simply hides the progress bar in the dialog. See gimp_file_dialog_progress_end().
Also, we have never heard of this before, so it seems fedora-specific. Are there any patches in the GIMP rpm?
We have two distro-specific patches that patch gimprc a bit: - use the system monitor profile by default - use the web browser for help (RHEL-only, we don't build GIMP's help browser there to not require webkitgtk2)
In comment i meant to say "...but that should NOT happen...". When looking at the traces, it seems this only happens for exports, so when the plug-in progress mechanism is involved. You have mentioned this above, but I just reviewed the entire plug-in progress and proc_frame code and couldn't see anything wrong.
Here's the current incarnation downstream: https://bugzilla.redhat.com/show_bug.cgi?id=1215905
And for reference, here are the patches we carry in Fedora for gimp and gtk2 respectively: http://pkgs.fedoraproject.org/cgit/gimp.git/tree/ http://pkgs.fedoraproject.org/cgit/gtk2.git/tree/
This should fix it, pushed to master and gimp-2-8: commit 908f4e69d656e5a1cd4b23310a16e287e8ae72f7 Author: Michael Natterer <mitch@gimp.org> Date: Tue Jul 14 13:22:06 2015 +0200 Bug 739003 - Crashes in file_save_dialog_response() Ref/unref the dialog around saving the image insatead of connecting to "destroy" and NULLifying the local dialog variable on destruction, which has caused weird crashes on fedora. (cherry picked from commit a0e48ad29e93e2dc259879abeb2dedc9a420b8f6) app/dialogs/file-save-dialog.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
Thanks for the fix! Meanwhile I got another report about the bug and the user described how to reproduce the crash: https://bugzilla.redhat.com/show_bug.cgi?id=1243033#c13 """ It seems 100% reproducible. Previously I wrote the reproducer from my head, so it may be not 100% correct, so how I am able to reproduce it now: - under xfce created screenshot by hitting PrintScreen - saved it to /tmp as .png - started gimp from terminal by typing 'gimp' - clicked file->open, navigated to /tmp and selected the screenshot - clicked file->export as - rewritten the name to end with .jpg - clicked export, export image as JPEG dialog opens - clicked X button on the main window of gimp (the one with the screenshot), the export dialog kept open - finished the export by clicking the export button on the export dialog Now it crashed after while, I don't need to click the X button of the empty main window, but I can. """ Mind that I could not reproduce it in SWM (because then the window contains more than just the image and doesn't get closed) and that you need to take care not to accidentally make a change to the image (otherwise the "There is one image with unsaved changes: ..." dialog will pop up and ruin the experience ;-)).
I just verified the build of 2.8.14 where I included the patch, and it doesn't crash but still throws a couple of warnings: --- 8< --- nils@gibraltar:~> gimp (gimp:2698): GLib-GObject-CRITICAL **: g_object_set_data_full: assertion 'G_IS_OBJECT (object)' failed (gimp:2698): Gimp-Core-CRITICAL **: gimp_image_flush: assertion 'GIMP_IS_IMAGE (image)' failed nils@gibraltar:~> --- >8 --- Do you think that we should look into this further?
Fixed the warnings by ref/unreffing dialog->image as well so that it gets kept around during saving the image: commit 0b900239d57161da11e3e0c8094552cf1480dcd3 Author: Nils Philippsen <nils@redhat.com> Date: Fri Jul 17 17:07:18 2015 +0200 Ref/unref dialog->image around saving as well... ...to avoid warnings that happen if the image got closed before the saving finishes. (amends commit a0e48ad29e93e2dc259879abeb2dedc9a420b8f6) app/dialogs/file-save-dialog.c | 2 ++ 1 file changed, 2 insertions(+)