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/inotify.go | 250 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 188 insertions(+), 62 deletions(-) (limited to 'inotify/inotify.go') 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 + } +} -- cgit v1.2.3