Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fuse: remove race conditions on fsh #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fuse: remove race conditions on fsh #53

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion fuse/host.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type FileSystemHost struct {
sigc chan os.Signal

capCaseInsensitive, capReaddirPlus, capDeleteAccess bool
sync.RWMutex
}

var (
Expand Down Expand Up @@ -421,6 +422,8 @@ func hostInit(conn0 *c_struct_fuse_conn_info) (user_data unsafe.Pointer) {
fctx := c_fuse_get_context()
user_data = fctx.private_data
host := hostHandleGet(user_data)
host.Lock()
defer host.Unlock()
host.fuse = fctx.fuse
c_hostAsgnCconninfo(conn0,
c_bool(host.capCaseInsensitive),
Expand All @@ -442,6 +445,8 @@ func hostDestroy(user_data unsafe.Pointer) {
}
host := hostHandleGet(user_data)
host.fsop.Destroy()
host.RLock()
defer host.RUnlock()
if nil != host.sigc {
signal.Stop(host.sigc)
}
Expand Down Expand Up @@ -676,7 +681,9 @@ func (host *FileSystemHost) Mount(mountpoint string, opts []string) bool {
}
}
defer func() {
host.Lock()
host.mntp = ""
host.Unlock()
}()

/*
Expand All @@ -696,7 +703,10 @@ func (host *FileSystemHost) Mount(mountpoint string, opts []string) bool {
host.sigc = make(chan os.Signal, 1)
defer close(host.sigc)
go func() {
_, ok := <-host.sigc
host.RLock()
sigc := host.sigc
host.RUnlock()
_, ok := <-sigc
if ok {
host.Unmount()
}
Expand All @@ -716,6 +726,8 @@ func (host *FileSystemHost) Mount(mountpoint string, opts []string) bool {
// Unmount may be called at any time after the Init() method has been called
// and before the Destroy() method has been called.
func (host *FileSystemHost) Unmount() bool {
host.RLock()
Copy link
Collaborator

@billziss-gh billziss-gh Jan 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The host.fuse variable is modified in hostInit and hostDestroy. The Unmount method is documented as being safe only between Init and Destroy, which means that the file system should provide its own synchronization method to ensure that Unmount is not called at the wrong time.

However I agree that it might be better if Unmount is improved to catch cases where the file system calls it before Init or after Destroy.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remove the lock here, I get into a race condition when executing go test -race; I had the same problem with my fs and I don't know how to fix it on the side of my fs.


if nil == host.fuse {
return false
}
Expand All @@ -724,12 +736,16 @@ func (host *FileSystemHost) Unmount() bool {
mntp = c_CString(host.mntp)
defer c_free(unsafe.Pointer(mntp))
}
host.RUnlock()
return 0 != c_hostUnmount(host.fuse, mntp)
}

// Notify notifies the operating system about a file change.
// The action is a combination of the fuse.NOTIFY_* constants.
func (host *FileSystemHost) Notify(path string, action uint32) bool {
host.RLock()
Copy link
Collaborator

@billziss-gh billziss-gh Jan 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments in Unmount apply here as well (especially because I failed to document that is only safe between Init and Destroy).

Copy link
Author

@btwotch btwotch Jan 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In comparison to func Unmount, it doesn't run into a race condition when running go test -race.

defer host.RUnlock()

if nil == host.fuse {
return false
}
Expand Down