|Summary:||start-here crashes sometimes|
|Product:||[Retired] Red Hat Public Beta||Reporter:||Havoc Pennington <hp>|
|Component:||gnome-vfs2||Assignee:||Havoc Pennington <hp>|
|Status:||CLOSED RAWHIDE||QA Contact:|
|Fixed In Version:||Doc Type:||Bug Fix|
|Doc Text:||Story Points:||---|
|Last Closed:||2003-02-24 19:26:25 UTC||Type:||---|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
|Cloudforms Team:||---||Target Upstream Version:|
|Bug Depends On:|
Description Havoc Pennington 2003-02-20 05:47:10 UTC
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.
Comment 1 Jonathan Blandford 2003-02-22 00:57:14 UTC
Comment 2 Jonathan Blandford 2003-02-22 01:50:32 UTC
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.
Comment 3 Jonathan Blandford 2003-02-22 01:51:44 UTC
Created attachment 90273 [details] Possible fix. Not pretty.
Comment 4 Alexander Larsson 2003-02-22 10:43:13 UTC
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.
Comment 5 Havoc Pennington 2003-02-22 15:08:46 UTC
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?
Comment 6 Alexander Larsson 2003-02-22 17:03:04 UTC
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.
Comment 7 Alexander Larsson 2003-02-22 17:04:03 UTC
Comment 8 Alexander Larsson 2003-02-22 17:08:59 UTC
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.
Comment 9 Alexander Larsson 2003-02-22 17:11:34 UTC
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.
Comment 10 Alexander Larsson 2003-02-22 17:17:58 UTC
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.
Comment 11 Alexander Larsson 2003-02-24 10:35:45 UTC
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.
Comment 12 Alexander Larsson 2003-02-24 10:44:45 UTC
Building this in gnome-vfs2-2.2.2-2.