From 09db31c7264e370fbbafab81a0c30b1ac1b80514 Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Sun, 18 Dec 2016 15:50:34 -0500 Subject: Make inotify.Inotify more robust; remove inotify.Watcher. BREAKING CHANGE. --- inotify/channels.go | 96 -------------------- inotify/inotify.go | 250 +++++++++++++++++++++++++++++++++++++++------------- inotify/syscall.go | 34 +++---- 3 files changed, 205 insertions(+), 175 deletions(-) delete mode 100644 inotify/channels.go diff --git a/inotify/channels.go b/inotify/channels.go deleted file mode 100644 index 737b312..0000000 --- a/inotify/channels.go +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright 2015 Luke Shumaker . -// -// This is free software; you can redistribute it and/or modify it -// under the terms of the GNU Lesser General Public License as -// published by the Free Software Foundation; either version 2.1 of -// the License, or (at your option) any later version. -// -// This software is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public -// License along with this manual; if not, see -// . - -package inotify - -import ( - "os" - "syscall" -) - -// A Watcher is a wrapper around an (*Inotify) that exposes a -// channel-based interface that is much nicer to work with. -type Watcher struct { - Events <-chan Event - Errors <-chan error - events chan<- Event - errors chan<- error - in *Inotify -} - -// Wraps InotifyInit() -func WatcherInit() (*Watcher, error) { - in, err := InotifyInit() - return newWatcher(in, err) -} - -// Wraps InotifyInit1() -func WatcherInit1(flags int) (*Watcher, error) { - in, err := InotifyInit1(flags &^ IN_NONBLOCK) - return newWatcher(in, err) -} - -func newWatcher(in *Inotify, err error) (*Watcher, error) { - events := make(chan Event) - errors := make(chan error) - o := &Watcher{ - Events: events, - events: events, - Errors: errors, - errors: errors, - in: in, - } - go o.worker() - return o, err -} - -// Wraps Inotify.AddWatch(); adds or modifies a watch. -func (o *Watcher) AddWatch(path string, mask Mask) (Wd, error) { - return o.in.AddWatch(path, mask) -} - -// Wraps Inotify.RmWatch(); removes a watch. -func (o *Watcher) RmWatch(wd Wd) error { - return o.in.RmWatch(wd) -} - -// Wraps Inotify.Close(). Unlike Inotify.Close(), -// this cannot block. Also unlike Inotify.Close(), nothing -// may be received from the channel after this is called. -func (o *Watcher) Close() { - func() { - defer recover() - close(o.events) - close(o.errors) - }() - go o.in.Close() -} - -func (o *Watcher) worker() { - defer recover() - for { - ev, err := o.in.Read() - if ev.Wd >= 0 { - o.events <- ev - } - if err != nil { - if err.(*os.SyscallError).Err == syscall.EBADF { - o.Close() - } - o.errors <- err - } - } -} diff --git a/inotify/inotify.go b/inotify/inotify.go index 2fd3a83..2db414c 100644 --- a/inotify/inotify.go +++ b/inotify/inotify.go @@ -1,4 +1,4 @@ -// Copyright 2015 Luke Shumaker . +// Copyright 2015-2016 Luke Shumaker . // // This is free software; you can redistribute it and/or modify it // under the terms of the GNU Lesser General Public License as @@ -22,16 +22,54 @@ import ( "sync" "syscall" "unsafe" + "os" ) +// Most of the complexity here is that we syncronize around access to +// the file descriptor. Why? Because we're worried that it could get +// closed and re-used between the time the fd is read from the os.File +// and the time the system call is actually dispatched. +// +// A B +// in.method() in.Close(); syscall.Open() +// --------------------------------------------------- +// fd = in.fd +// syscall.Close() +// syscall.Open() +// syscall(fd) +// +// And you're wondering "should we really protoect against that; after +// all: share by communicating, don't communicate by sharing" and "why +// do we have to deal with this condition if os.File doesn't?" +// Because an inotify instance is more like a net.Listener. If you +// notice, net.netFD has do deal with a very similar situtation. At +// some level, you may simply observe that with inotify it makes sense +// to have one goroutine add/remove watches, and have another read +// them; where this kind of communication doesn't make sense with +// ordinary files. +// +// So, how does net.netFD deal with it? Well... it is tightly coupled +// with some private routines in runtime/sema.go. That is, we can't +// look at it for too much guidance. + type Inotify struct { - fd file - fdLock sync.RWMutex - buffFull [4096]byte + fd inFd + fdLock sync.RWMutex + fdBlock bool + + buffFull [4096]byte // 4KiB is a good size, but the bare + // minimum is `sizeof(struct + // inotify_event) + NAME_MAX + 1` buff []byte buffLock sync.Mutex + buffErr error + + ch chan chev } +// Event is a file system event from inotify. +// +// An Event is invalid if Wd is < 0. type Event struct { Wd Wd // Watch descriptor Mask Mask // Mask describing event @@ -39,90 +77,178 @@ type Event struct { Name *string // Optional name } -// Create an inotify instance. The variant InotifyInit1() allows -// flags to access extra functionality. -func InotifyInit() (*Inotify, error) { - fd, err := inotify_init() - o := Inotify{ +type chev struct { + Ev Event + Err error +} + +func newInotify(fd inFd, blocking bool) *Inotify { + if fd < 0 { + return nil + } + in := &Inotify{ fd: fd, + fdBlock: blocking, + ch: make(chan chev), } - o.buff = o.buffFull[:0] - return &o, err + in.buff = in.buffFull[:0] + go in.worker() + return in } -// Create an inotify instance, with flags specifying extra -// functionality. +// InotifyInit creates an inotify instance. The variant +// InotifyInit1() allows flags to access extra functionality. +func InotifyInit() (*Inotify, error) { + fd, err := sys_inotify_init() + return newInotify(fd, true), err +} + +// InotifyInit1 create an inotify instance, with flags specifying +// extra functionality. func InotifyInit1(flags int) (*Inotify, error) { - fd, err := inotify_init1(flags) - o := Inotify{ - fd: fd, - } - o.buff = o.buffFull[:0] - return &o, err + fd, err := sys_inotify_init1(flags &^ IN_NONBLOCK) + return newInotify(fd, flags & IN_NONBLOCK == 0), err } -// Add a watch to the inotify instance, or modifies an existing watch -// item. -func (o *Inotify) AddWatch(path string, mask Mask) (Wd, error) { - o.fdLock.RLock() - defer o.fdLock.RUnlock() - return inotify_add_watch(o.fd, path, mask) +// AddWatch adds a watch to the inotify instance, or modifies an +// existing watch item. +func (in *Inotify) AddWatch(path string, mask Mask) (Wd, error) { + in.fdLock.RLock() + defer in.fdLock.RUnlock() + return sys_inotify_add_watch(in.fd, path, mask) } -// Remove a watch from the inotify instance. -func (o *Inotify) RmWatch(wd Wd) error { - o.fdLock.RLock() - defer o.fdLock.RUnlock() - return inotify_rm_watch(o.fd, wd) +// RmWatch removes a watch from the inotify instance. +func (in *Inotify) RmWatch(wd Wd) error { + in.fdLock.RLock() + defer in.fdLock.RUnlock() + return sys_inotify_rm_watch(in.fd, wd) } -// Close the inotify instance; further calls to this object will -// error. -// -// Events recieved before Close() is called may still be Read() after -// the call to Close(). -// -// Beware that if Close() is called while waiting on Read(), it will -// block until events are read. -func (o *Inotify) Close() error { - o.fdLock.Lock() - defer o.fdLock.Unlock() - defer func() { o.fd = -1 }() - return sysclose(o.fd) +// Close closes the inotify instance; further calls to this object +// will error. +func (in *Inotify) Close() (err error) { + defer func() { + if r := recover(); r != nil { + // This is a double-close condition. + // + // Choices for the error: + // - Linux: EBADF + // - os.File: syscall.EINVAL + // - net.netFD: net.errClosing = errors.New(...) + err = os.NewSyscallError("close", syscall.EBADF) + } + }() + close(in.ch) // will panic if already closed; hence above + + // I would love to be able to return the error from sys_close; + // but it might block forever. + go func() { + in.fdLock.Lock() + in.fdLock.Unlock() + sys_close(in.fd) + }() + return } -// Read an event from the inotify instance. +// read is a low-level read an event from the inotify instance. // -// Events recieved before Close() is called may still be Read() after -// the call to Close(). -func (o *Inotify) Read() (Event, error) { - o.buffLock.Lock() - defer o.buffLock.Unlock() - - if len(o.buff) == 0 { - o.fdLock.RLock() - len, err := sysread(o.fd, o.buffFull[:]) - o.fdLock.RUnlock() - if len == 0 { - return Event{Wd: -1}, o.Close() - } else if len < 0 { - return Event{Wd: -1}, err +// It's low-level/private because it can deadlock between it and the +// `go` block in Close. Instead, a worker calls this in a loop, +// writes the result to a channel, and a public reader can safely get +// it from the channel. No deadlock. +func (in *Inotify) read() (Event, error) { + in.buffLock.Lock() + defer in.buffLock.Unlock() + + if len(in.buff) == 0 { + if in.buffErr != nil { + return Event{Wd: -1}, in.buffErr } - o.buff = o.buffFull[0:len] + var n int + in.fdLock.RLock() + n, in.buffErr = sys_read(in.fd, in.buffFull[:]) + in.fdLock.RUnlock() + in.buff = in.buffFull[0:n] } - raw := (*syscall.InotifyEvent)(unsafe.Pointer(&o.buff[0])) + if len(in.buff) < syscall.SizeofInotifyEvent { + // Either Linux screwed up (and we have no chance of + // handling that sanely), or this Inotify came from an + // existing FD that wasn't really an inotify instance. + in.buffErr = syscall.EBADF + return Event{Wd: -1}, in.buffErr + } + raw := (*syscall.InotifyEvent)(unsafe.Pointer(&in.buff[0])) ret := Event{ Wd: Wd(raw.Wd), Mask: Mask(raw.Mask), Cookie: raw.Cookie, Name: nil, } + if int64(len(in.buff)) < syscall.SizeofInotifyEvent+int64(raw.Len) { + // Same as above. + in.buffErr = syscall.EBADF + return Event{Wd: -1}, in.buffErr + } if raw.Len > 0 { - bytes := (*[syscall.NAME_MAX]byte)(unsafe.Pointer(&o.buff[syscall.SizeofInotifyEvent])) + bytes := (*[syscall.NAME_MAX]byte)(unsafe.Pointer(&in.buff[syscall.SizeofInotifyEvent])) name := string(bytes[:raw.Len-1]) ret.Name = &name } - o.buff = o.buff[0 : syscall.SizeofInotifyEvent+raw.Len] + in.buff = in.buff[0 : syscall.SizeofInotifyEvent+raw.Len] return ret, nil } + +func (in *Inotify) worker() { + defer recover() + for { + ev, err := in.read() + in.ch <- chev{ev, err} // will panic on .Close() + } +} + +// Read calls either ReadBlock or ReadNonblock, depending on whether +// the Inotify instance has the IN_NONBLOCK flag set. +func (in *Inotify) Read() (Event, error) { + if in.fdBlock { + return in.ReadBlock() + } else { + return in.ReadNonblock() + } +} + +// ReadBlock reads exactly one Event or an error from the inotify +// instance. If an Event or error is not available for reading, it +// blocks until one is. +// +// If err is non-nil, then the Event is invalid (ev.Wd < 0); and if +// err is nil, then then the Event is valid. +func (in *Inotify) ReadBlock() (Event, error) { + e, ok := <-in.ch + if !ok { + return Event{Wd: -1}, os.NewSyscallError("read", syscall.EBADF) + } + return e.Ev, e.Err +} + +// ReadNonblock reads either an Event or an error from the inotify +// instance, but doesn't block if an event isn't ready. +// +// Unlike Read, it may be the case that both the Event is invalid and +// the error is nil. This indicates that the read simply returned +// instead of blocking. +// +// ev, err := in.ReadNonblock() +// haveRead = ev.Wd >= 0 || err != nil +func (in *Inotify) ReadNonblock() (Event, error) { + select { + case e, ok := <-in.ch: + if !ok { + return Event{Wd: -1}, os.NewSyscallError("read", syscall.EBADF) + } + return e.Ev, e.Err + default: + return Event{Wd: -1}, nil + } +} diff --git a/inotify/syscall.go b/inotify/syscall.go index d1b5140..0a0c2f0 100644 --- a/inotify/syscall.go +++ b/inotify/syscall.go @@ -1,4 +1,4 @@ -// Copyright 2015 Luke Shumaker . +// Copyright 2015-2016 Luke Shumaker . // // This is free software; you can redistribute it and/or modify it // under the terms of the GNU Lesser General Public License as @@ -29,35 +29,35 @@ func newPathError(op string, path string, err error) error { } // Create and initialize inotify instance. -func inotify_init() (file, error) { - fd, errno := syscall.InotifyInit() - return file(fd), os.NewSyscallError("inotify_init", errno) +func sys_inotify_init() (inFd, error) { + fd, err := syscall.InotifyInit() + return inFd(fd), os.NewSyscallError("inotify_init", err) } // Create and initialize inotify instance. -func inotify_init1(flags int) (file, error) { - fd, errno := syscall.InotifyInit1(flags) - return file(fd), os.NewSyscallError("inotify_init1", errno) +func sys_inotify_init1(flags int) (inFd, error) { + fd, err := syscall.InotifyInit1(flags) + return inFd(fd), os.NewSyscallError("inotify_init1", err) } // Add watch of object NAME to inotify instance FD. Notify about // events specified by MASK. -func inotify_add_watch(fd file, name string, mask Mask) (Wd, error) { - wd, errno := syscall.InotifyAddWatch(int(fd), name, uint32(mask)) - return Wd(wd), newPathError("inotify_add_watch", name, errno) +func sys_inotify_add_watch(fd inFd, name string, mask Mask) (Wd, error) { + wd, err := syscall.InotifyAddWatch(int(fd), name, uint32(mask)) + return Wd(wd), newPathError("inotify_add_watch", name, err) } // Remove the watch specified by WD from the inotify instance FD. -func inotify_rm_watch(fd file, wd Wd) error { - success, errno := syscall.InotifyRmWatch(int(fd), uint32(wd)) +func sys_inotify_rm_watch(fd inFd, wd Wd) error { + success, err := syscall.InotifyRmWatch(int(fd), uint32(wd)) switch success { case -1: - if errno == nil { + if err == nil { panic("should never happen") } - os.NewSyscallError("inotify_rm_watch", errno) + os.NewSyscallError("inotify_rm_watch", err) case 0: - if errno != nil { + if err != nil { panic("should never happen") } return nil @@ -65,11 +65,11 @@ func inotify_rm_watch(fd file, wd Wd) error { panic("should never happen") } -func sysclose(fd file) error { +func sys_close(fd inFd) error { return os.NewSyscallError("close", syscall.Close(int(fd))) } -func sysread(fd file, p []byte) (int, error) { +func sys_read(fd inFd, p []byte) (int, error) { n, err := syscall.Read(int(fd), p) return n, os.NewSyscallError("read", err) } -- cgit v1.2.3-2-g168b