GNOME Bugzilla – Bug 551337
gedit crashes browsing fileselector ssh locations
Last modified: 2010-11-25 17:37:03 UTC
the bug has been opened on https://bugs.launchpad.net/bugs/236953 "When using gedit to browse a mounted sftp location for a file to open, or when browsing the sftp location to save a file, gedit will crash when using the breadcrumb (history) buttons to navigate. How to replicate: 1) Add a bookmark to an ssh location which contains a couple of layers of directories 2) Open gedit and choose "File>Open..." 3) Click on the new bookmark and navigate down a directory tree a couple of levels 4) Use the breadcrumb buttons to go back to the top level of the bookmark 5) If gedit hasn't already crashed just navigate back and forth a couple of times using the buttons, it invariably does. This also happens on "Save" and "Save As.." which can lead to losing work. Updated 7th Sept 2008 to clarify and include steps to reproduce."
ubuntu comments show those warnings: "** (gedit:10379): WARNING **: Operation not supported by backend gedit: ../../src/xcb_lock.c:77: _XGetXCBBuffer: Assertion `((int) ((xcb_req) - (dpy->request)) >= 0)' failed."
This seems now fixed by calling gdk_threads_init... gtkfilechooser user threads internally to interact with gtk, so even if gedit itself doesn't use threads the function is needed.
After some discussion or irc it turns out that calling gdk_threads_init is just a workaround, but the bug is really in gvfs that is calling callbacks from threads that are not the main thread. The crash is triggered by the fact that gtk is trying to set the X cursor from multiple threads. This in turn is due from the fact that gvfs is running callback from the wrong threads. The attached debugging patch for gtk shows it clearly and leads to the following stack trace: Gtk-ERROR **: Unexpected call from a different thread aborting... Program received signal SIGABRT, Aborted.
+ Trace 206511
Thread 3038239632 (LWP 27283)
Here are some hints given on irc, but obviously it needs someone actually knowing that code to figure out the right fix... <hp> the fix _could_ be changing g_simple_async_result_complete to g_simple_async_result_complete_in_idle <hp> then again, it could not be that <owen> Looks like that code should be changed to g_main_context_iteration() to me <owen> that is, if you want to sleep for incoming messages in a thread, you need to just run the GLib main loop and let that pick up DBus messages and dispatch them <owen> or maybe it needs ot be switched to a private connection <owen> Or maybe the connection is private, and g_simple_async_result_complete_in_idle is the right answer :-) <owen> Looks like the connectin is meant to be private <owen> pbor: Anyways, I think if you put that stack trace in a gvfs bug report (or reassign the existing one) it should be easily fixable by someone who understands the gvfs code <pbor> owen: yeah, I'll do that, thank you <owen> I'ts clear that a lot of thought went into getting this right in gvfs, and there's just some leakage here
Created attachment 118328 [details] [review] patch to point out the issue
Ah, this seems like a tricky issue. The problem is in the implementation of the gvfs next_file() method in that it pumps the sync dbus connection to wait for incomming calls. However for the async emulation of this we do that in another thread than the one the sync dbus connection is for, so we reply to other dbus messages in the wrong thread. I'll try to figure out a fix for this.
The easiest workaround for now will be to just make set_busy_cursor defer to an idle when called from those callbacks.
That doesn't sound very good to me: the fact that async gvfs calls run callback in the wrong thread will surely cause other bugs in other places and evry time it will be a PITA to reproduce and figure out. We should really fix gvfs instead of patching this symptom
Created attachment 118595 [details] [review] possible patch Does this patch fix the crashes ?
Well, the filechooser code is a bit careless anyway. I'm pretty sure there are places where set_busy_cursor is called outside the gdk lock, even with the gvfs problem fixed.
I stll manage to make gedit (with the gdk_threads_init workaround removed) crash even with the patch applied. I get this: Gtk:ERROR:gtkcontainer.c:2673:IA__gtk_container_propagate_expose: assertion failed: (child->parent == GTK_WIDGET (container)) Probably there are other things that do not like to be called from different threads beside set_busy_cursor (even if that one was prolly the most frequent, I took a bit more to get the crash now). If you think that the patch is needed anyway, it looks ok to me... just a question (because I am a bit ignorant about main loop internals): when running the filechooser in a recursive main loop (modal), does g_source_attach (NULL) attach it to the inner main loop?
The docs say It is possible to create new instances of GMainLoop recursively. This is often used in GTK+ applications when showing modal dialog boxes. Note that event sources are associated with a particular GMainContext, and will be checked and dispatched for all main loops associated with that GMainContext. It would be great to get a stacktrace from the crashes you still see.
Ok, here are the stack traces for crashes I managed to trigger even with the patch applied. The second one is particularly interesting since it still crashing in set_busy_cursor, so it means there is something wrong with the patch. sys:1: GtkWarning: gtk_tree_model_sort_convert_iter_to_child_iter: assertion `GTK_IS_TREE_MODEL_SORT (tree_model_sort)' failed Program received signal SIGSEGV, Segmentation fault.
+ Trace 206755
Thread 3070043904 (LWP 8684)
detail=0, var_args=0xbfa8aed0 "
I know what the core problem and the fix is, I'll try to schedule some time tomorrow to write the fix.
...would be great because HardCode Freeze is tomorrow (Monday 23:59 UTC).
2008-09-15 Alexander Larsson <alexl@redhat.com> * client/gdaemonfile.c: * client/gdaemonfileenumerator.c: Add proper implementation of async file enumeration. This avoids the problem with the default thread based ones that could cause callbacks in the non-mainloop thread. (#551337) Checked in on the 2.24 branch. It fixes the problem for me. Could you also test it?
I couldn't get the filechooser to crash with the updated gvfs. Thanks Alex. Maybe it's worth adding a unit test that checks if the callback is run in the main thread? Just a thought...
paolo: Its kinda hard to trigger the case where this would happen as its somewhat of a race. So the unit test might not be so easy to write.
Moving back to gtk for remaining possible set_busy_cursor issues
I checked gtkfilesystem.c and all the invocations of user callbacks are wrapped with gdk_threads_enter/leave(). I checked gtkfilechooserdefault.c to see where set_busy_cursor() is called, and it's random functions or those async callbacks that do the wrapping. Can we close this now?
(In reply to comment #19) > I checked gtkfilesystem.c and all the invocations of user callbacks are wrapped > with gdk_threads_enter/leave(). > > I checked gtkfilechooserdefault.c to see where set_busy_cursor() is called, and > it's random functions or those async callbacks that do the wrapping. > > Can we close this now? can we close?
Let's close!