Red Hat Bugzilla – Bug 84668
start-here crashes sometimes
Last modified: 2008-05-01 11:38:05 EDT
A couple reports of crashes when opening start-here, some sort of gnome-vfs race. Need to get symbols installed and click on it a few times and get backtraces.
see also: http://bugzilla.gnome.org/show_bug.cgi?id=106758 http://bugzilla.gnome.org/show_bug.cgi?id=106760
I have a really really ugly patch to fix this. I think. I can't actually reproduce the bug here, but this makes sense given the backtrace. Alex, can you look at this and tell me what you think? It seems equally evil, but might help.
Created attachment 90273 [details] Possible fix. Not pretty.
I don't like that fix at all. I think its better to acquire the handle_hash lock earlier in gnome_vfs_monitor_do_add(). I.E. lock it before the call to uri->method->monitor_add(). This guarantees that we don't look it up in the hash before its there. It also removes the race jrb comments on in his patch.
Just for background, our bug theory here is that monitor_add() calls into fam, and then calls back out into the _callback() function before the hash_table_insert(), and then callback() crashes because the hash entry isn't there. If you add the lock before the monitor_add(), we need to know that monitor_add() can't result in invoking callback() from the same thread that's doing the monitor_add(), because it would deadlock; we weren't sure about that so made the safest change. There is *no* time for testing of this change... are you confident that monitor_add() is not going to try to take the lock?
I guess that could happen. But then jrbs patch is broken too. It would get stuck in a loop waiting for the handle to be added to the hash table, which won't happen until it returns... Maybe the safest thing is to detact this case and print a warning but ignore it. Loosing a change notification is not that bad.
eh, s/detact/detect/.
OTOH. I can't see a way to get monitor_add to call gnome_vfs_monitor_callback() in the two existing implemenations of monitor_add.
What I assumed happened was that the monitor was added, but then the callback happened on another thread which was switched to before the handle was added to the hash table.
Ah, the file-method.c implementation can call *a* callback from monitor_add. Not for the added monitor though, but for another previously added one.
Created attachment 90306 [details] new patch This is the patch I'm building with. Its basically jrbs patch, but with a warning if you destroy a monitor that hasn't been added to the hashtable yet, and it doesn't drop+reaquire the lock when the lookup succeeds. Please review this.
Building this in gnome-vfs2-2.2.2-2.