Skip to content

Commit

Permalink
go.exp/fsnotify: remove current implementation of WatchFlags
Browse files Browse the repository at this point in the history
removes fsnFlags, purgeEvents and internalEvent channel

The current implementation:
* doesn't take advantage of OS for efficiency
* no benefit over filtering events as received
* extra bookkeeping and mutexes
* no tests
* not correctly implemented on Windows howeyc/fsnotify#93 (comment)

LGTM=mikioh.mikioh, rsc
R=golang-codereviews, mikioh.mikioh, rsc
CC=golang-codereviews
https://golang.org/cl/100860043
  • Loading branch information
Nathan John Youngman authored and cixtor committed Jun 20, 2014
1 parent 296a214 commit d9e1c40
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 153 deletions.
64 changes: 0 additions & 64 deletions fsnotify/fsnotify.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,77 +7,13 @@ package fsnotify

import "fmt"

const (
FSN_CREATE = 1
FSN_MODIFY = 2
FSN_DELETE = 4
FSN_RENAME = 8

FSN_ALL = FSN_MODIFY | FSN_DELETE | FSN_RENAME | FSN_CREATE
)

// Purge events from interal chan to external chan if passes filter
func (w *Watcher) purgeEvents() {
for ev := range w.internalEvent {
sendEvent := false
w.fsnmut.Lock()
fsnFlags := w.fsnFlags[ev.Name]
w.fsnmut.Unlock()

if (fsnFlags&FSN_CREATE == FSN_CREATE) && ev.IsCreate() {
sendEvent = true
}

if (fsnFlags&FSN_MODIFY == FSN_MODIFY) && ev.IsModify() {
sendEvent = true
}

if (fsnFlags&FSN_DELETE == FSN_DELETE) && ev.IsDelete() {
sendEvent = true
}

if (fsnFlags&FSN_RENAME == FSN_RENAME) && ev.IsRename() {
sendEvent = true
}

if sendEvent {
w.Event <- ev
}

// If there's no file, then no more events for user
// BSD must keep watch for internal use (watches DELETEs to keep track
// what files exist for create events)
if ev.IsDelete() {
w.fsnmut.Lock()
delete(w.fsnFlags, ev.Name)
w.fsnmut.Unlock()
}
}

close(w.Event)
}

// Watch a given file path
func (w *Watcher) Watch(path string) error {
w.fsnmut.Lock()
w.fsnFlags[path] = FSN_ALL
w.fsnmut.Unlock()
return w.watch(path)
}

// Watch a given file path for a particular set of notifications (FSN_MODIFY etc.)
func (w *Watcher) WatchFlags(path string, flags uint32) error {
w.fsnmut.Lock()
w.fsnFlags[path] = flags
w.fsnmut.Unlock()
return w.watch(path)
}

// Remove a watch on a file
func (w *Watcher) RemoveWatch(path string) error {
w.fsnmut.Lock()
delete(w.fsnFlags, path)
w.fsnmut.Unlock()
return w.removeWatch(path)
}

Expand Down
30 changes: 3 additions & 27 deletions fsnotify/fsnotify_bsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ type Watcher struct {
kq int // File descriptor (as returned by the kqueue() syscall)
watches map[string]int // Map of watched file descriptors (key: path)
wmut sync.Mutex // Protects access to watches.
fsnFlags map[string]uint32 // Map of watched files to flags used for filter
fsnmut sync.Mutex // Protects access to fsnFlags.
enFlags map[string]uint32 // Map of watched files to evfilt note flags used in kqueue
enmut sync.Mutex // Protects access to enFlags.
paths map[int]string // Map of watched paths (key: watch descriptor)
Expand All @@ -75,7 +73,6 @@ type Watcher struct {
externalWatches map[string]bool // Map of watches added by user of the library.
ewmut sync.Mutex // Protects access to externalWatches.
Error chan error // Errors are sent on this channel
internalEvent chan *FileEvent // Events are queued on this channel
Event chan *FileEvent // Events are returned on this channel
done chan bool // Channel for sending a "quit message" to the reader goroutine
isClosed bool // Set to true when Close() is first called
Expand All @@ -92,20 +89,17 @@ func NewWatcher() (*Watcher, error) {
w := &Watcher{
kq: fd,
watches: make(map[string]int),
fsnFlags: make(map[string]uint32),
enFlags: make(map[string]uint32),
paths: make(map[int]string),
finfo: make(map[int]os.FileInfo),
fileExists: make(map[string]bool),
externalWatches: make(map[string]bool),
internalEvent: make(chan *FileEvent),
Event: make(chan *FileEvent),
Error: make(chan error),
done: make(chan bool, 1),
}

go w.readEvents()
go w.purgeEvents()
return w, nil
}

Expand Down Expand Up @@ -323,7 +317,7 @@ func (w *Watcher) readEvents() {
if errno != nil {
w.Error <- os.NewSyscallError("close", errno)
}
close(w.internalEvent)
close(w.Event)
close(w.Error)
return
}
Expand Down Expand Up @@ -369,7 +363,7 @@ func (w *Watcher) readEvents() {
w.sendDirectoryChangeEvents(fileEvent.Name)
} else {
// Send the event on the events channel
w.internalEvent <- fileEvent
w.Event <- fileEvent
}

// Move to next event
Expand Down Expand Up @@ -419,15 +413,6 @@ func (w *Watcher) watchDirectoryFiles(dirPath string) error {
for _, fileInfo := range files {
filePath := filepath.Join(dirPath, fileInfo.Name())

// Inherit fsnFlags from parent directory
w.fsnmut.Lock()
if flags, found := w.fsnFlags[dirPath]; found {
w.fsnFlags[filePath] = flags
} else {
w.fsnFlags[filePath] = FSN_ALL
}
w.fsnmut.Unlock()

if fileInfo.IsDir() == false {
// Watch file to mimic linux fsnotify
e := w.addWatch(filePath, sys_NOTE_ALLEVENTS)
Expand Down Expand Up @@ -477,20 +462,11 @@ func (w *Watcher) sendDirectoryChangeEvents(dirPath string) {
_, doesExist := w.fileExists[filePath]
w.femut.Unlock()
if !doesExist {
// Inherit fsnFlags from parent directory
w.fsnmut.Lock()
if flags, found := w.fsnFlags[dirPath]; found {
w.fsnFlags[filePath] = flags
} else {
w.fsnFlags[filePath] = FSN_ALL
}
w.fsnmut.Unlock()

// Send create event
fileEvent := new(FileEvent)
fileEvent.Name = filePath
fileEvent.create = true
w.internalEvent <- fileEvent
w.Event <- fileEvent
}
w.femut.Lock()
w.fileExists[filePath] = true
Expand Down
54 changes: 17 additions & 37 deletions fsnotify/fsnotify_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// +build linux

package fsnotify

import (
Expand Down Expand Up @@ -93,17 +91,14 @@ type watch struct {
}

type Watcher struct {
mu sync.Mutex // Map access
fd int // File descriptor (as returned by the inotify_init() syscall)
watches map[string]*watch // Map of inotify watches (key: path)
fsnFlags map[string]uint32 // Map of watched files to flags used for filter
fsnmut sync.Mutex // Protects access to fsnFlags.
paths map[int]string // Map of watched paths (key: watch descriptor)
Error chan error // Errors are sent on this channel
internalEvent chan *FileEvent // Events are queued on this channel
Event chan *FileEvent // Events are returned on this channel
done chan bool // Channel for sending a "quit message" to the reader goroutine
isClosed bool // Set to true when Close() is first called
mu sync.Mutex // Map access
fd int // File descriptor (as returned by the inotify_init() syscall)
watches map[string]*watch // Map of inotify watches (key: path)
paths map[int]string // Map of watched paths (key: watch descriptor)
Error chan error // Errors are sent on this channel
Event chan *FileEvent // Events are returned on this channel
done chan bool // Channel for sending a "quit message" to the reader goroutine
isClosed bool // Set to true when Close() is first called
}

// NewWatcher creates and returns a new inotify instance using inotify_init(2)
Expand All @@ -113,18 +108,15 @@ func NewWatcher() (*Watcher, error) {
return nil, os.NewSyscallError("inotify_init", errno)
}
w := &Watcher{
fd: fd,
watches: make(map[string]*watch),
fsnFlags: make(map[string]uint32),
paths: make(map[int]string),
internalEvent: make(chan *FileEvent),
Event: make(chan *FileEvent),
Error: make(chan error),
done: make(chan bool, 1),
fd: fd,
watches: make(map[string]*watch),
paths: make(map[int]string),
Event: make(chan *FileEvent),
Error: make(chan error),
done: make(chan bool, 1),
}

go w.readEvents()
go w.purgeEvents()
return w, nil
}

Expand Down Expand Up @@ -210,7 +202,7 @@ func (w *Watcher) readEvents() {
select {
case <-w.done:
syscall.Close(w.fd)
close(w.internalEvent)
close(w.Event)
close(w.Error)
return
default:
Expand All @@ -221,7 +213,7 @@ func (w *Watcher) readEvents() {
// If EOF is received
if n == 0 {
syscall.Close(w.fd)
close(w.internalEvent)
close(w.Event)
close(w.Error)
return
}
Expand Down Expand Up @@ -252,7 +244,6 @@ func (w *Watcher) readEvents() {
w.mu.Lock()
event.Name = w.paths[int(raw.Wd)]
w.mu.Unlock()
watchedName := event.Name
if nameLen > 0 {
// Point "bytes" at the first byte of the filename
bytes := (*[syscall.PathMax]byte)(unsafe.Pointer(&buf[offset+syscall.SizeofInotifyEvent]))
Expand All @@ -262,18 +253,7 @@ func (w *Watcher) readEvents() {

// Send the events that are not ignored on the events channel
if !event.ignoreLinux() {
// Setup FSNotify flags (inherit from directory watch)
w.fsnmut.Lock()
if _, fsnFound := w.fsnFlags[event.Name]; !fsnFound {
if fsnFlags, watchFound := w.fsnFlags[watchedName]; watchFound {
w.fsnFlags[event.Name] = fsnFlags
} else {
w.fsnFlags[event.Name] = FSN_ALL
}
}
w.fsnmut.Unlock()

w.internalEvent <- event
w.Event <- event
}

// Move to the next event in the buffer
Expand Down
42 changes: 17 additions & 25 deletions fsnotify/fsnotify_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// +build windows

package fsnotify

import (
Expand Down Expand Up @@ -115,18 +113,15 @@ type watchMap map[uint32]indexMap
// A Watcher waits for and receives event notifications
// for a specific set of files and directories.
type Watcher struct {
mu sync.Mutex // Map access
port syscall.Handle // Handle to completion port
watches watchMap // Map of watches (key: i-number)
fsnFlags map[string]uint32 // Map of watched files to flags used for filter
fsnmut sync.Mutex // Protects access to fsnFlags.
input chan *input // Inputs to the reader are sent on this channel
internalEvent chan *FileEvent // Events are queued on this channel
Event chan *FileEvent // Events are returned on this channel
Error chan error // Errors are sent on this channel
isClosed bool // Set to true when Close() is first called
quit chan chan<- error
cookie uint32
mu sync.Mutex // Map access
port syscall.Handle // Handle to completion port
watches watchMap // Map of watches (key: i-number)
input chan *input // Inputs to the reader are sent on this channel
Event chan *FileEvent // Events are returned on this channel
Error chan error // Errors are sent on this channel
isClosed bool // Set to true when Close() is first called
quit chan chan<- error
cookie uint32
}

// NewWatcher creates and returns a Watcher.
Expand All @@ -136,17 +131,14 @@ func NewWatcher() (*Watcher, error) {
return nil, os.NewSyscallError("CreateIoCompletionPort", e)
}
w := &Watcher{
port: port,
watches: make(watchMap),
fsnFlags: make(map[string]uint32),
input: make(chan *input, 1),
Event: make(chan *FileEvent, 50),
internalEvent: make(chan *FileEvent),
Error: make(chan error),
quit: make(chan chan<- error, 1),
port: port,
watches: make(watchMap),
input: make(chan *input, 1),
Event: make(chan *FileEvent, 50),
Error: make(chan error),
quit: make(chan chan<- error, 1),
}
go w.readEvents()
go w.purgeEvents()
return w, nil
}

Expand Down Expand Up @@ -431,7 +423,7 @@ func (w *Watcher) readEvents() {
if e := syscall.CloseHandle(w.port); e != nil {
err = os.NewSyscallError("CloseHandle", e)
}
close(w.internalEvent)
close(w.Event)
close(w.Error)
ch <- err
return
Expand Down Expand Up @@ -475,7 +467,7 @@ func (w *Watcher) readEvents() {
var offset uint32
for {
if n == 0 {
w.internalEvent <- &FileEvent{mask: sys_FS_Q_OVERFLOW}
w.Event <- &FileEvent{mask: sys_FS_Q_OVERFLOW}
w.Error <- errors.New("short read in readEvents()")
break
}
Expand Down

0 comments on commit d9e1c40

Please sign in to comment.