GNOME Bugzilla – Bug 520903
Hangs on filechooser error
Last modified: 2008-03-10 16:59:05 UTC
1. Open eog, and press Ctrl+O 2. Type "foobarmagic/" in the path bar, then "enter" 3. Dismiss the error dialogue 4. See it's hanging Works in Totem, file-roller or the gnome-screenshooter.
This happens with the filechooser backend set to "gio" (/desktop/gnome/interface/file_chooser_backend).
Hmm, Helgrind gives this once the error dialog appears: ==5594== Thread #1 unlocked a not-locked lock at 0x6089648 ==5594== at 0x402812F: pthread_mutex_unlock (in /usr/lib/valgrind/x86-linux/vgpreload_helgrind.so) ==5594== by 0x498CC2B: gdk_threads_impl_unlock (gdk.c:395) ==5594== by 0x46D04FD: gtk_dialog_run (gtkdialog.c:1040) ==5594== by 0x46EC767: error_message (gtkfilechooserdefault.c:1042) ==5594== by 0x46EC7DA: error_dialog (gtkfilechooserdefault.c:1083) ==5594== by 0x46EC8BF: error_changing_folder_dialog (gtkfilechooserdefault.c:1168) ==5594== by 0x46F2161: update_current_folder_get_info_cb (gtkfilechooserdefault.c:7006) ==5594== by 0x7F839E0: query_info_callback (gtkfilesystemgio.c:857) ==5594== by 0x41E00F0: g_simple_async_result_complete (gsimpleasyncresult.c:553) ==5594== by 0x41E015D: complete_in_idle_cb (gsimpleasyncresult.c:563) ==5594== by 0x51F8AA0: g_idle_dispatch (gmain.c:4143) ==5594== by 0x51FA654: g_main_context_dispatch (gmain.c:2065) ==5594== by 0x51FD9D6: g_main_context_iterate (gmain.c:2698) ==5594== by 0x51FDD66: g_main_loop_run (gmain.c:2906) ==5594== by 0x47558D2: gtk_main (gtkmain.c:1163) ==5594== by 0x8060BE5: main (main.c:223) ==5594== Lock at 0x6089648 was first observed ==5594== at 0x40298FD: pthread_mutex_init (in /usr/lib/valgrind/x86-linux/vgpreload_helgrind.so) ==5594== by 0x4606370: g_mutex_new_posix_impl (gthread-posix.c:163) ==5594== by 0x498CFB7: gdk_threads_init (gdk.c:414) ==5594== by 0x8060B29: main (main.c:208) ==5594== ==5594== Thread #1: Attempt to re-lock a non-recursive lock I already hold ==5594== at 0x4027D3F: pthread_mutex_lock (in /usr/lib/valgrind/x86-linux/vgpreload_helgrind.so) ==5594== by 0x498CBDB: gdk_threads_impl_lock (gdk.c:388) ==5594== by 0x46FED03: check_completion_callback (gtkfilechooserentry.c:504) ==5594== by 0x51A8CA6: source_closure_marshal_BOOLEAN__VOID (gsourceclosure.c:81) ==5594== by 0x518F7C7: g_closure_invoke (gclosure.c:490) ==5594== by 0x51A8C54: source_closure_callback (gsourceclosure.c:123) ==5594== by 0x51F8AA0: g_idle_dispatch (gmain.c:4143) ==5594== by 0x51FA654: g_main_context_dispatch (gmain.c:2065) ==5594== by 0x51FD9D6: g_main_context_iterate (gmain.c:2698) ==5594== by 0x51FDD66: g_main_loop_run (gmain.c:2906) ==5594== by 0x47558D2: gtk_main (gtkmain.c:1163) ==5594== by 0x8060BE5: main (main.c:223)
Created attachment 106780 [details] test-tool exposing the deadlock This test tool shows that every application using gdk_threads_* could become a victim of this. Compile with: gcc -O2 `pkg-config --libs --cflags gtk+-2.0 gthread-2.0` to see the deadlock Compile with gcc `pkg-config --libs --cflags gtk+-2.0 gthread-2.0` -DWITHOUT_GDK_THREADS to disable gdk_threads_* and don't see the deadlock.
The test script in comment 3 show that this is not an EOG specific problem as it is triggered by using gdk_threads_{init,enter,leave} (if I've done nothing wrong). So I am reassigning this to libgnomeui for inspection for now.
Given that there's no threading code whatsoever in the backend, it looks like a GTK+ bug.
(In reply to comment #5) > Given that there's no threading code whatsoever in the backend, ... > Could be this is actually the bug. I have no idea how that GtkFileSystem stuff fully works but I have compared the standard GTK version and the gnome-vfs version with the gio one. The gnome-vfs filesystem backend wraps most callbacks in gdk_threads_{enter,leave} calls while GtkFileSystemUnix seems to have something that looks like a callback dispatcher executing them using gdk_threads_add_idle. Wrapping query_info_callback() in the gio backend with _enter and _leave seems to fix the deadlock.
Marking as blocker, to give this as much visibility as possible.. (from #gnome-hackers: (18:15:35) claudio: andre: not sure if it's a showstopper, but it is worth giving some visibility so that someone checks if felix is right (18:15:53) claudio: so that it gets fixed before the release (18:17:31) andre: claudio, hah, the release is on monday and i don't think there will be a gtk+ release on monday... (18:19:58) andre: claudio, you could poke kris in #gtk or so... don't know if they really care about blocker bugs or not... but i'd appreciate if you would set it to blocker state and make some noise :) (18:27:59) claudio: andre: ok, I'll mark it as blocker, but I can't really make much more noise than this, no internet connection at home, and luckily I'm in a starbucks atm.)
I don't know how I checked, but it's a bug in the GIO backend alright.
Created attachment 106937 [details] [review] libgnomeui-gio-fix-threading.patch Enclose in gdk_threads_* calls all the callbacks and signal emissions from async responses. Fixes the bug for me. Kjartan, could you please push this patch to the release team and get it into tomorrow's releases?
FYI, I took the freedom to nag the Release Team on behalf of whoever should have done it. So now, there are two approvals, but one with the condition that two people have tested the patch and we're sure that it won't break something else. This is the thread: http://mail.gnome.org/archives/release-team/2008-March/msg00133.html
I tested it, in file-roller, Totem and eog. (EOG being the one that had problems). Seems to work fine for me. Anyone else doing the testing?
I tested it with file-roller, eog and my test-tool from above. The patch fixes the deadlock mentioned here as well as two other I could find. The first one being an error message if you try to access your floppy drive without disk in it. This displays a strange error message from HAL which was even able to deadlock while still showing the error dialog. The second one is the file overwrite confirmation if you try to overwrite an already existing file. All are fixed by Bastien's patch. :-) Apps not using gdk_threads_init() shouldn't notice the patch as gdk_threads_{lock,unlock} are NOPs there.
Found one problem now. Creating a new folder will deadlock now. But this easily fixable by taking out the gdk_threads_{enter,leave} calls in create_folder_callback(). These are unnecessary as the callback is executed by gdk_threads_add_idle which makes sure all locks are locked beforehand and unlocked afterwards.
Like this you mean: Index: gtkfilesystemgio.c =================================================================== --- gtkfilesystemgio.c (revisjon 5576) +++ gtkfilesystemgio.c (arbeidskopi) @@ -1044,11 +1044,8 @@ g_file_make_directory (file, handle->cancellable, &error); - gdk_threads_enter (); ((GtkFileSystemCreateFolderCallback) handle->callback) (GTK_FILE_SYSTEM_HANDLE (handle), idle_data->path, error, handle->data); - gdk_threads_leave (); - g_object_unref (file); gtk_file_path_free (idle_data->path); g_slice_free (CreateFolderData, idle_data);
(In reply to comment #14) > Like this you mean: > ... > Yes. That's what I meant.
2.22.01 is out with this fix.
(In reply to comment #13) > Found one problem now. > > Creating a new folder will deadlock now. > But this easily fixable by taking out the gdk_threads_{enter,leave} calls in > create_folder_callback(). These are unnecessary as the callback is executed by > gdk_threads_add_idle which makes sure all locks are locked beforehand and > unlocked afterwards. My mistake, I thought it was an async callback from gio. Thanks for the testing/committing guys.