GNOME Bugzilla – Bug 553995
evolution crashed with SIGSEGV in gtkhtml_editor_set_changed()
Last modified: 2013-09-13 01:00:14 UTC
this bug has been filed here: https://bugs.edge.launchpad.net/ubuntu/+source/evolution/+bug/274823 "Had just saved and closed a draft message. Evolution was just open and idle." ".
+ Trace 207413
Thread 1 (process 6734)
0) Ran into the same issue. Backtrace from coredump is identical: (gdb) bt
+ Trace 208321
html = (GtkHTML *) 0x0 __PRETTY_FUNCTION__ = "gtkhtml_editor_set_changed" 1) Relevant code: (gdb) l 1024,1042 1024 void 1025 gtkhtml_editor_set_changed (GtkhtmlEditor *editor, 1026 gboolean changed) 1027 { 1028 GtkHTML *html; 1029 1030 /* XXX GtkHTML does not notify us when its internal "saved" state 1031 * changes, so we can't have a "changed" property because its 1032 * notifications would be unreliable. */ 1033 1034 g_return_if_fail (GTKHTML_IS_EDITOR (editor)); 1035 1036 html = gtkhtml_editor_get_html (editor); 1037 1038 if (!changed) 1039 html_engine_saved (html->engine); 1040 1041 editor->priv->changed = changed; 1042 } 2) Seems like gtkhtml_editor_set_changed() needs to check for a NULL return from gtkhtml_editor_get_html(). Anyway, reclassify as a GtkHtml bug?
0) Basically just pinging here (because I just ran into this again on 2.25.2). Identical backtrace, so no need to past that one here. 1) Again: toss over the wall to gtkhtml?
Hi Paul, try to find why is the 'html' NULL. If you can reproduce it reliably, then it's a bit easier than without reproducer. (I know it doesn't help sometimes, but usually it does.) :) Also, where is your draft folder, local or remote? There can be some issue with remote folders because of bug #552583. Other thing is that this runs asynchronously, thus if save is slow, and you are quick enough to close composer before the save finishes, then it can crash (I'm guessing only, I didn't check the code in question yet).
(In reply to comment #3) > try to find why is the 'html' NULL. If you can reproduce it reliably, > then it's a bit easier than without reproducer. (I know it doesn't help > sometimes, but usually it does.) :) That just requires me to recompile evolution and gtkhtml? with -O0 and -g and try ? number of times. No biggie, it seems I just have to discover what causes a race in a multithreaded application. Seriously, is there some way to have gdb barf whenever a specific instance of a variable (html in this case) is written to (preferably only if a '0' is written to it)? > Also, where is your draft folder, local or remote? Remote (IMAP). Note that the draft was successfully saved before the crash.
(In reply to comment #4) > Seriously, is there some way to have gdb barf whenever a specific instance of a > variable (html in this case) is written to (preferably only if a '0' is written > to it)? Yeah, you can set a GDB watchpoint to break when a particular address or expression changes ("watch" is the command). But there's a secret GLib trick that might work better for this case. It only works with some architectures, but I assume you're on an i386. - Recompile GLib with --enable-debug. - In GDB, set the variable "g_trap_object_ref" to the address of the GObject you want to monitor (the "html" variable in this case). You'll have to set a breakpoint at some strategic place in the code to learn the object address. I'd suggest gtkhtml_editor_get_html(). - Continue. GDB will now break everytime the object is referenced or unreferenced. You can check the reference count each time and see if the object is being destroyed too early. Note, it breaks immediately _before_ the reference count changes. So if you break in g_object_unref() and the reference count is 1, that means the object is about to be destroyed.
0) (In reply to comment #3) > try to find why is the 'html' NULL. If you can reproduce it reliably, > then it's a bit easier than without reproducer. Can now reproduce it reliably. Just as the reporter said: - save draft - then close the editor (and do that quite quickly) 1) A rough analysis (reading the code in the back trace, not even bothered yet to recompile evolution and/or related stuff), written down to aid my memory: i) evolution is running some callback triggered by the save of the draft, i.e: mail-mt.c:mail_msg_idle_cb () mail-ops.c:append_mail_done () em-composer-utils.c:save_draft_done () em-composer-utils.c:composer_set_no_change () gtkhtml-editor.c:gtkhtml_editor_set_changed () ii) while that is happening (slowly because of IMAP round trips or whatever) the "composer window" is closed, probably deleting lots of stuff with it, and that makes "html" now referring to something already deleted. 2) Not sure what happens in detail. Nowhere near an idea how to fix this race (it must be a race). Maybe (some of) the code triggered on closing the composer window needs to block on the save callback to be finished. But how and where?
Created attachment 124397 [details] [review] proposed patch against gtkhtm-3.25.2 0) Did a lot of reading of code and chatting on IRC. I even read some documentation on GObject. 1) Long story short: what seems to be going on is that gtkhtml_editor_set_changed() is called in the window where gtkhtml_editor_private_dispose() is called (which disposes editor->priv->edit_area, which is the pointer gtkhtml_editor_get_html() gets for us) but the code that finalizes the relevant editor is not yet called. gtkhtml_editor_set_changed() doesn't handle that gracefully, which is a documented no-no: "When dispose ends, the object should not hold any reference to any other member object. The object is also expected to be able to answer client method invocations (with possibly an error code but no memory violation) until finalize is executed." 2) Can't really prove my theory, but with this patch against gtkhtml I wasn't able to reproduce this crash any more. 3) Not much further research went into this patch: it just tries to fix this bug. Other bugs of the same category will be fixed when we go along ("Let's fix this here and see where things will break then ..."). 4) Note that all my work of the last few days brought me exactly where fifteen minutes of work already brought me two months ago.
Looks good, feel free to commit to trunk/stable, with appropriate changelog entry.
committed to trunk (revision 9062): http://svn.gnome.org/viewvc/gtkhtml?view=revision&sortby=date&revision=9062 committed to stable (revision 9063): http://svn.gnome.org/viewvc/gtkhtml?view=revision&sortby=date&revision=9063
I added a comment to that function explaining why the check is necessary. I remember having trouble getting the reference counting right in that scenario when I first wrote the editor code. It might be that we just need to ref the editor until the draft is saved. I guess we can revisit this if problems continue to crop up.