Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 84668

Summary: start-here crashes sometimes
Product: [Retired] Red Hat Public Beta Reporter: Havoc Pennington <hp>
Component: gnome-vfs2Assignee: Havoc Pennington <hp>
Status: CLOSED RAWHIDE QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: phoebeCC: alexl, jrb
Target Milestone: ---Keywords: Triaged
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2003-02-24 19:26:25 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 79578    
Attachments:
Description Flags
Possible fix. Not pretty.
none
new patch none

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.