GNOME Bugzilla – Bug 723366
GtkFileChooser crashes due to a signal connection left behind by GtkMountOperation
Last modified: 2014-05-06 10:05:48 UTC
Created attachment 267729 [details] The relevant part of Valgrind log Our users and I observed application crashes in the following scenario: 1. The system has a volume (partition, connected USB stick, whatever else) that is not mounted at the moment. 2. Launch smplayer (the bug may trigger with other apps as well). 3. Select "Open" => "File...". The dialog appears. 4. Click on the volume label in the dialog for the volume to be mounted. 5. Select a file on the volume. 6. Click "Open". GTK+ 2.24.20 OS: ROSA Fresh R2 GNOME x64 This issue does not happen every time. It is a memory corruption, the Valgrind log is attached. I have investigated the problem. Looks like the following happens there. GtkMountOperation is created to perform the mount. gtk_mount_operation_set_parent() is called for it, making it a child of the dialog window. That function connects "destroy" signal for the parent with gtk_widget_destroyed() callback passing the address of GtkMountOperation::priv::parent_window as a parameter (gtkmountoperation.c:1435). After mount operation is complete, GtkMountOperation instance is destroyed but the signal connection persists. When the dialog is closed, it triggers, gtk_widget_destroyed() is called and writes NULL to the memory previously occupied by GtkMountOperation::priv::parent_window. This way, some other structure may be corrupted if the memory was given to that structure after deletion of GtkMountOperation. In our case, GtkFileSystemModel was corrupted. Removing that signal connection in gtk_mount_operation_finalize() fixes that problem for me. I will attach the patch a bit later.
Created attachment 267730 [details] [review] Proposed patch The proposed patch to fix the problem.
Review of attachment 267730 [details] [review]: With the comment removed, looks fine to commit. Thanks for the investigation ! ::: gtk+-2.24.20.orig/gtk/gtkmountoperation.c @@ +200,3 @@ + * destroyed and gtk_widget_destroyed() will write NULL to the memory + * previously occupied by 'priv->parent_window'. This could lead to + * crashes. */ The comment is not necessary here - severing such connections in this place is quite common and doesn't need an extra explanation.
(In reply to comment #2) > The comment is not necessary here - severing such connections in this place is > quite common and doesn't need an extra explanation. I see. Yes, it is OK to remove the comment.
Hey folks, first off, thanks a *lot* for investigating this and fixing it. We've received so many bugreports against Thunar because of this bug (it segfaults when unmounting volumes). @mclasen: Could you please also apply this patch to Gtk2? Thanks in advance!