Bug 84668 - start-here crashes sometimes
Summary: start-here crashes sometimes
Alias: None
Product: Red Hat Public Beta
Classification: Retired
Component: gnome-vfs2
Version: phoebe
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Havoc Pennington
QA Contact:
Depends On:
Blocks: 79578
TreeView+ depends on / blocked
Reported: 2003-02-20 05:47 UTC by Havoc Pennington
Modified: 2008-05-01 15:38 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2003-02-24 19:26:25 UTC

Attachments (Terms of Use)
Possible fix. Not pretty. (972 bytes, patch)
2003-02-22 01:51 UTC, Jonathan Blandford
no flags Details | Diff
new patch (2.00 KB, patch)
2003-02-24 10:35 UTC, Alexander Larsson
no flags Details | Diff

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 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
eh, s/detact/detect/.

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.

Note You need to log in before you can comment on or make changes to this bug.