Fixed potential incomplete read of inotify events in case of many con…

…current events with long (more than 16 chars) file names; Fixed potential missing events; Fixed 0 reads caused by select() fired with unset fdset; Fixed potential context leaks

dirwatcher.go

@@ -24,6 +24,7 @@ func NewDirWatcher(ctx context.Context, fileMask uint32, root string) (*DirWatch
 
 	i, err := NewInotify(ctx)
 	if err != nil {
+		cancel()
 		return nil, err
 	}
 

filewatcher.go

@@ -40,6 +40,7 @@ func NewFileWatcher(ctx context.Context, mask uint32, files ...string) (*FileWat
 	events := make(chan FileEvent)
 
 	go func() {
+		defer cancel()
 		for {
 			raw, err := inotify.Read()
 
@@ -61,6 +62,7 @@ func NewFileWatcher(ctx context.Context, mask uint32, files ...string) (*FileWat
 	}()
 
 	go func() {
+		defer cancel()
 		for {
 			select {
 			case <-ctx.Done():

inotify.go

@@ -14,6 +14,9 @@ import (
 	"unsafe"
 )
 
+// max number of events to read at once
+const maxEvents = 1024
+
 var TimeoutError = errors.New("Inotify timeout")
 
 type getWatchRequest struct {
@@ -184,15 +187,15 @@ func (i *Inotify) Read() ([]InotifyEvent, error) {
 // ReadDeadline waits for InotifyEvents until deadline is reached, or context is cancelled. If
 // deadline is reached, TimeoutError is returned.
 func (i *Inotify) ReadDeadline(deadline time.Time) ([]InotifyEvent, error) {
-	events := make([]InotifyEvent, 0, 1024)
-	buf := make([]byte, 1024*(syscall.SizeofInotifyEvent+16))
+	events := make([]InotifyEvent, 0, maxEvents)
+	buf := make([]byte, maxEvents*(syscall.SizeofInotifyEvent+syscall.NAME_MAX+1))
 
 	var n int
 	var err error
 
 	fdset := &syscall.FdSet{}
-	fdset.Bits[0] = 1 << uint(i.fd)
 
+	//main:
 	for {
 		if i.ctx.Err() != nil {
 			return events, i.ctx.Err()
@@ -208,6 +211,7 @@ func (i *Inotify) ReadDeadline(deadline time.Time) ([]InotifyEvent, error) {
 
 		timeout := syscall.NsecToTimeval(diff.Nanoseconds())
 
+		fdset.Bits[0] = 1 << uint(i.fd)
 		_, err = syscall.Select(i.fd+1, fdset, nil, nil, &timeout)
 
 		if err != nil {
@@ -217,6 +221,10 @@ func (i *Inotify) ReadDeadline(deadline time.Time) ([]InotifyEvent, error) {
 			return events, err
 		}
 
+		if fdset.Bits[0]&(1<<uint(i.fd)) == 0 {
+			continue // No data to read, continue waiting
+		}
+
 		n, err = syscall.Read(i.fd, buf)
 		if err != nil {
 			if err == syscall.EAGAIN {
@@ -231,19 +239,26 @@ func (i *Inotify) ReadDeadline(deadline time.Time) ([]InotifyEvent, error) {
 	}
 
 	if n < syscall.SizeofInotifyEvent {
-		return events, fmt.Errorf("Short inotify read")
+		return events, fmt.Errorf("short inotify read, expected at least one SizeofInotifyEvent %d, got %d", syscall.SizeofInotifyEvent, n)
 	}
 
 	offset := 0
 
 	for offset+syscall.SizeofInotifyEvent <= n {
 
 		event := (*syscall.InotifyEvent)(unsafe.Pointer(&buf[offset]))
-		namebuf := buf[offset+syscall.SizeofInotifyEvent : offset+syscall.SizeofInotifyEvent+int(event.Len)]
+		var name string
+		{
+			nameStart := offset + syscall.SizeofInotifyEvent
+			nameEnd := offset + syscall.SizeofInotifyEvent + int(event.Len)
 
-		offset += syscall.SizeofInotifyEvent + int(event.Len)
+			if nameEnd > n {
+				return events, fmt.Errorf("corrupted inotify event length %d", event.Len)
+			}
 
-		name := strings.TrimRight(string(namebuf), "\x00")
+			name = strings.TrimRight(string(buf[nameStart:nameEnd]), "\x00")
+			offset = nameEnd
+		}
 
 		req := getPathRequest{
 			wd:     uint32(event.Wd),

inotify_test.go

@@ -7,6 +7,8 @@ import (
 	"os"
 	"path/filepath"
 	"runtime/pprof"
+	"strings"
+	"syscall"
 	"testing"
 	"time"
 )
@@ -160,6 +162,53 @@ func TestInotify(t *testing.T) {
 		}
 	})
 
+	// This test should generate more events than the buffer passed to syscall.Read()
+	// can handle. This is to test the buffer handling in the ReadDeadline() method.
+	// The potential bug is that the buffer may contain patial event at the end of the buffer
+	// and the next syscall.Read() will not be able to read the rest of the event.
+	t.Run("MultipleEvents #2 - Reading leftover events", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(ctx)
+		defer cancel()
+
+		i, err := NewInotify(ctx)
+		if err != nil {
+			t.Error(err)
+		}
+
+		i.AddWatch(dir, IN_CLOSE_WRITE)
+
+		// generate 2* maxEvents events with long filenames (in range from syscall.NAME_MAX-10 to syscall.NAME_MAX)
+		for x := 0; x < 2*maxEvents; x++ {
+			fileNameLen := syscall.NAME_MAX - 10 + x%10
+			// make a filename with len = fileNameLen
+			fileName := fmt.Sprintf("%s-%d", "hz", x)
+			fileName = fmt.Sprintf("%s%s", fileName, strings.Repeat("a", fileNameLen-len(fileName)+1))
+
+			f, err := os.OpenFile(filepath.Join(dir, fileName), os.O_RDWR|os.O_CREATE, 0)
+			if err != nil {
+				t.Error(err)
+			}
+			f.Close()
+		}
+
+		// read all events
+		events, err := i.Read()
+		if err != nil {
+			t.Error(err)
+		}
+
+		events2, err := i.Read()
+		if err != nil {
+			t.Error(err)
+		}
+
+		// check if all events were read
+		if len(events)+len(events2) != 2*maxEvents {
+			t.Errorf("Expected %d events, but got %d", 2*maxEvents, len(events))
+		}
+
+	})
+
 	t.Run("SelfFolderEvent", func(t *testing.T) {
 		ctx, cancel := context.WithCancel(ctx)
 		defer cancel()