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.
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.
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]
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.