From b31d51e0b16f6233f2285adbab674ecfa2ace4ff Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Sat, 5 Sep 2015 13:59:36 -0600 Subject: inotify: Avoid most of the race conditions, get rid of Cint There's still a condition that could be a race with fd-reuse, if one goroutine is calling inotify.{AddWatch,RmWatch,Read}(); another goroutine is calling inotify.Close(), and several things happen between loadFd() running and the add_watch/rm_watch/read syscall launching: - syscall.Close() returns - syscall.Open() reuses the filedescriptor A B syscall(loadFd()) inotify.Close(); syscall.Open() ---------------------------------------------------------- loadFd() syscall.Close() syscall.Open() syscall() Given that Read() can't be allowed to block Close() from running, I'm not sure there's a way to fix this. --- inotify/bits.go | 15 ++++++++++++++- inotify/inotify.go | 50 ++++++++++++++++++-------------------------------- inotify/syscall.go | 29 +++++++++++++---------------- 3 files changed, 45 insertions(+), 49 deletions(-) diff --git a/inotify/bits.go b/inotify/bits.go index 3606b52..9162435 100644 --- a/inotify/bits.go +++ b/inotify/bits.go @@ -1,5 +1,7 @@ package inotify +import "sync/atomic" + const ( // Flags for the parameter of InotifyInit1(). // These, oddly, appear to be 24-bit numbers. @@ -7,8 +9,19 @@ const ( IN_NONBLOCK uint32 = 00004000 ) -type Mask uint32 +// Logically, Fd and Wd should be 'int', not 'int64', to match the OS. +// But, because there's no 'sync/atomic.SwapInt', we cast up to int64. +type Wd int64 +type Fd int64 + +func swapFd(addr *Fd, new Fd) (old Fd) { + return Fd(atomic.SwapInt64((*int64)(addr), int64(new))) +} +func loadFd(addr *Fd) Fd { + return Fd(atomic.LoadInt64((*int64)(addr))) +} +type Mask uint32 const ( // Supported events suitable for the `mask` parameter of Inotify.AddWatch(). IN_ACCESS Mask = (1<< 0) // File was accessed. diff --git a/inotify/inotify.go b/inotify/inotify.go index 6d74830..f7db1b0 100644 --- a/inotify/inotify.go +++ b/inotify/inotify.go @@ -1,23 +1,21 @@ package inotify import ( - "errors" "syscall" "unsafe" + "sync" ) -var InotifyAlreadyClosedError error = errors.New("inotify instance already closed") - type Inotify struct { - fd Cint - isClosed bool + fd Fd fullbuff [4096]byte buff []byte + buffLock sync.Mutex } type Event struct { - Wd Cint /* Watch descriptor */ + Wd Wd /* Watch descriptor */ Mask Mask /* Mask describing event */ Cookie uint32 /* Unique cookie associating related events (for rename(2)) */ Name *string /* Optional name */ @@ -26,54 +24,42 @@ type Event struct { func InotifyInit() (*Inotify, error) { fd, err := inotify_init() o := Inotify{ - fd: Cint(fd), - isClosed: false, + fd: fd, } o.buff = o.fullbuff[:0] return &o, err } -func InotifyInit1(flags Cint) (*Inotify, error) { +func InotifyInit1(flags int) (*Inotify, error) { fd, err := inotify_init1(flags) o := Inotify{ - fd: Cint(fd), - isClosed: false, + fd: fd, } o.buff = o.fullbuff[:0] return &o, err } -func (o *Inotify) AddWatch(path string, mask Mask) (Cint, error) { - if o.isClosed { - return -1, InotifyAlreadyClosedError - } - return inotify_add_watch(o.fd, path, uint32(mask)) +func (o *Inotify) AddWatch(path string, mask Mask) (Wd, error) { + return inotify_add_watch(loadFd(&o.fd), path, mask) } -func (o *Inotify) RmWatch(wd Cint) error { - if o.isClosed { - return InotifyAlreadyClosedError - } - return inotify_rm_watch(o.fd, wd) +func (o *Inotify) RmWatch(wd Wd) error { + return inotify_rm_watch(loadFd(&o.fd), wd) } func (o *Inotify) Close() error { - if o.isClosed { - return InotifyAlreadyClosedError - } - o.isClosed = true - return sysclose(o.fd) + return sysclose(swapFd(&o.fd, -1)) } func (o *Inotify) Read() (Event, error) { + o.buffLock.Lock() + defer o.buffLock.Unlock() + if len(o.buff) == 0 { - if o.isClosed { - return Event{Wd: -1}, InotifyAlreadyClosedError - } - len, err := sysread(o.fd, o.buff) + len, err := sysread(loadFd(&o.fd), o.buff) if len == 0 { return Event{Wd: -1}, o.Close() - } else if len <= 0 { + } else if len < 0 { return Event{Wd: -1}, err } o.buff = o.fullbuff[0:len] @@ -81,7 +67,7 @@ func (o *Inotify) Read() (Event, error) { raw := (*syscall.InotifyEvent)(unsafe.Pointer(&o.buff[0])) ret := Event{ - Wd: Cint(raw.Wd), + Wd: Wd(raw.Wd), Mask: Mask(raw.Mask), Cookie: raw.Cookie, Name: nil, diff --git a/inotify/syscall.go b/inotify/syscall.go index 1b5c426..721a10a 100644 --- a/inotify/syscall.go +++ b/inotify/syscall.go @@ -1,14 +1,11 @@ package inotify import ( - "C" "os" "syscall" ) -type Cint C.int - -func pathError(op string, path string, err error) error { +func newPathError(op string, path string, err error) error { if err == nil { return nil } @@ -16,26 +13,26 @@ func pathError(op string, path string, err error) error { } /* Create and initialize inotify instance. */ -func inotify_init() (Cint, error) { +func inotify_init() (Fd, error) { fd, errno := syscall.InotifyInit() - return Cint(fd), os.NewSyscallError("inotify_init", errno) + return Fd(fd), os.NewSyscallError("inotify_init", errno) } /* Create and initialize inotify instance. */ -func inotify_init1(flags Cint) (Cint, error) { - fd, errno := syscall.InotifyInit1(int(flags)) - return Cint(fd), os.NewSyscallError("inotify_init1", errno) +func inotify_init1(flags int) (Fd, error) { + fd, errno := syscall.InotifyInit1(flags) + return Fd(fd), os.NewSyscallError("inotify_init1", errno) } /* Add watch of object NAME to inotify instance FD. Notify about events specified by MASK. */ -func inotify_add_watch(fd Cint, name string, mask uint32) (Cint, error) { - wd, errno := syscall.InotifyAddWatch(int(fd), name, mask) - return Cint(wd), pathError("inotify_add_watch", name, errno) +func inotify_add_watch(fd Fd, name string, mask Mask) (Wd, error) { + wd, errno := syscall.InotifyAddWatch(int(fd), name, uint32(mask)) + return Wd(wd), newPathError("inotify_add_watch", name, errno) } /* Remove the watch specified by WD from the inotify instance FD. */ -func inotify_rm_watch(fd Cint, wd Cint) error { +func inotify_rm_watch(fd Fd, wd Wd) error { success, errno := syscall.InotifyRmWatch(int(fd), uint32(wd)) switch success { case -1: @@ -52,11 +49,11 @@ func inotify_rm_watch(fd Cint, wd Cint) error { panic("should never happen") } -func sysclose(fd Cint) error { +func sysclose(fd Fd) error { return os.NewSyscallError("close", syscall.Close(int(fd))) } -func sysread(fd Cint, p []byte) (Cint, error) { +func sysread(fd Fd, p []byte) (int, error) { n, err := syscall.Read(int(fd), p) - return Cint(n), os.NewSyscallError("read", err) + return n, os.NewSyscallError("read", err) } -- cgit v1.2.3-54-g00ecf