]> go.fuhry.dev Git - fsnotify.git/commitdiff
kqueue: fix Remove-logic
authorBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Sun, 10 Jan 2016 15:04:37 +0000 (16:04 +0100)
committerNathan Youngman <git@nathany.com>
Thu, 14 Jan 2016 02:10:36 +0000 (19:10 -0700)
`os.Lstat` does not return `error` when a file/directory exists,
so the code path  to check for newly created files after a REMOVE
on BSD was never executed.

Also, if the watched entry is a file, it has been removed from the watcher, so do a new-file-check for them as well.

This commit fixes issues on TextMate on OS X and others that uses `exchangedata` to do atomic saves.

See #17. closes #111.

CHANGELOG.md
integration_darwin_test.go [new file with mode: 0644]
kqueue.go

index da6aa282e78d59438f861a045c34346bb541694f..d37247e0b5f5fca7e5abdf05302e3869871f5e92 100644 (file)
@@ -1,5 +1,9 @@
 # Changelog
 
+## v1.2.9 / 2016-01-13
+
+kqueue: Fix logic for CREATE after REMOVE [#111](https://github.com/go-fsnotify/fsnotify/pull/111) (thanks @bep)
+
 ## v1.2.8 / 2015-12-17
 
 * kqueue: fix race condition in Close [#105](https://github.com/go-fsnotify/fsnotify/pull/105) (thanks @djui for reporting the issue and @ppknap for writing a failing test)
diff --git a/integration_darwin_test.go b/integration_darwin_test.go
new file mode 100644 (file)
index 0000000..1b8982f
--- /dev/null
@@ -0,0 +1,146 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package fsnotify
+
+import (
+       "os"
+       "path/filepath"
+       "syscall"
+       "testing"
+       "time"
+)
+
+// testExchangedataForWatcher tests the watcher with the exchangedata operation on OS X.
+//
+// This is widely used for atomic saves on OS X, e.g. TextMate and in Apple's NSDocument.
+//
+// See https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man2/exchangedata.2.html
+// Also see: https://github.com/textmate/textmate/blob/cd016be29489eba5f3c09b7b70b06da134dda550/Frameworks/io/src/swap_file_data.cc#L20
+func testExchangedataForWatcher(t *testing.T, watchDir bool) {
+       // Create directory to watch
+       testDir1 := tempMkdir(t)
+
+       // For the intermediate file
+       testDir2 := tempMkdir(t)
+
+       defer os.RemoveAll(testDir1)
+       defer os.RemoveAll(testDir2)
+
+       resolvedFilename := "TestFsnotifyEvents.file"
+
+       // TextMate does:
+       //
+       // 1. exchangedata (intermediate, resolved)
+       // 2. unlink intermediate
+       //
+       // Let's try to simulate that:
+       resolved := filepath.Join(testDir1, resolvedFilename)
+       intermediate := filepath.Join(testDir2, resolvedFilename+"~")
+
+       // Make sure we create the file before we start watching
+       createAndSyncFile(t, resolved)
+
+       watcher := newWatcher(t)
+
+       // Test both variants in isolation
+       if watchDir {
+               addWatch(t, watcher, testDir1)
+       } else {
+               addWatch(t, watcher, resolved)
+       }
+
+       // Receive errors on the error channel on a separate goroutine
+       go func() {
+               for err := range watcher.Errors {
+                       t.Fatalf("error received: %s", err)
+               }
+       }()
+
+       // Receive events on the event channel on a separate goroutine
+       eventstream := watcher.Events
+       var removeReceived counter
+       var createReceived counter
+
+       done := make(chan bool)
+
+       go func() {
+               for event := range eventstream {
+                       // Only count relevant events
+                       if event.Name == filepath.Clean(resolved) {
+                               if event.Op&Remove == Remove {
+                                       removeReceived.increment()
+                               }
+                               if event.Op&Create == Create {
+                                       createReceived.increment()
+                               }
+                       }
+                       t.Logf("event received: %s", event)
+               }
+               done <- true
+       }()
+
+       // Repeat to make sure the watched file/directory "survives" the REMOVE/CREATE loop.
+       for i := 1; i <= 3; i++ {
+               // The intermediate file is created in a folder outside the watcher
+               createAndSyncFile(t, intermediate)
+
+               // 1. Swap
+               if err := syscall.Exchangedata(intermediate, resolved, 0); err != nil {
+                       t.Fatalf("[%d] exchangedata failed: %s", i, err)
+               }
+
+               time.Sleep(50 * time.Millisecond)
+
+               // 2. Delete the intermediate file
+               err := os.Remove(intermediate)
+
+               if err != nil {
+                       t.Fatalf("[%d] remove %s failed: %s", i, intermediate, err)
+               }
+
+               time.Sleep(50 * time.Millisecond)
+
+       }
+
+       // We expect this event to be received almost immediately, but let's wait 500 ms to be sure
+       time.Sleep(500 * time.Millisecond)
+
+       // The events will be (CHMOD + REMOVE + CREATE) X 2. Let's focus on the last two:
+       if removeReceived.value() < 3 {
+               t.Fatal("fsnotify remove events have not been received after 500 ms")
+       }
+
+       if createReceived.value() < 3 {
+               t.Fatal("fsnotify create events have not been received after 500 ms")
+       }
+
+       watcher.Close()
+       t.Log("waiting for the event channel to become closed...")
+       select {
+       case <-done:
+               t.Log("event channel closed")
+       case <-time.After(2 * time.Second):
+               t.Fatal("event stream was not closed after 2 seconds")
+       }
+}
+
+// TestExchangedataInWatchedDir test exchangedata operation on file in watched dir.
+func TestExchangedataInWatchedDir(t *testing.T) {
+       testExchangedataForWatcher(t, true)
+}
+
+// TestExchangedataInWatchedDir test exchangedata operation on watched file.
+func TestExchangedataInWatchedFile(t *testing.T) {
+       testExchangedataForWatcher(t, false)
+}
+
+func createAndSyncFile(t *testing.T, filepath string) {
+       f1, err := os.OpenFile(filepath, os.O_WRONLY|os.O_CREATE, 0666)
+       if err != nil {
+               t.Fatalf("creating %s failed: %s", filepath, err)
+       }
+       f1.Sync()
+       f1.Close()
+}
index 7a7700bac7bb61173a1a2b14bc4e1f850de05986..b8ea30846d7e2ad84688c63667a86ca1d2f99949 100644 (file)
--- a/kqueue.go
+++ b/kqueue.go
@@ -325,19 +325,24 @@ func (w *Watcher) readEvents() {
                        if event.Op&Remove == Remove {
                                // Look for a file that may have overwritten this.
                                // For example, mv f1 f2 will delete f2, then create f2.
-                               fileDir, _ := filepath.Split(event.Name)
-                               fileDir = filepath.Clean(fileDir)
-                               w.mu.Lock()
-                               _, found := w.watches[fileDir]
-                               w.mu.Unlock()
-                               if found {
-                                       // make sure the directory exists before we watch for changes. When we
-                                       // do a recursive watch and perform rm -fr, the parent directory might
-                                       // have gone missing, ignore the missing directory and let the
-                                       // upcoming delete event remove the watch from the parent directory.
-                                       if _, err := os.Lstat(fileDir); os.IsExist(err) {
-                                               w.sendDirectoryChangeEvents(fileDir)
-                                               // FIXME: should this be for events on files or just isDir?
+                               if path.isDir {
+                                       fileDir := filepath.Clean(event.Name)
+                                       w.mu.Lock()
+                                       _, found := w.watches[fileDir]
+                                       w.mu.Unlock()
+                                       if found {
+                                               // make sure the directory exists before we watch for changes. When we
+                                               // do a recursive watch and perform rm -fr, the parent directory might
+                                               // have gone missing, ignore the missing directory and let the
+                                               // upcoming delete event remove the watch from the parent directory.
+                                               if _, err := os.Lstat(fileDir); err == nil {
+                                                       w.sendDirectoryChangeEvents(fileDir)
+                                               }
+                                       }
+                               } else {
+                                       filePath := filepath.Clean(event.Name)
+                                       if fileInfo, err := os.Lstat(filePath); err == nil {
+                                               w.sendFileCreatedEventIfNew(filePath, fileInfo)
                                        }
                                }
                        }
@@ -407,24 +412,35 @@ func (w *Watcher) sendDirectoryChangeEvents(dirPath string) {
        // Search for new files
        for _, fileInfo := range files {
                filePath := filepath.Join(dirPath, fileInfo.Name())
-               w.mu.Lock()
-               _, doesExist := w.fileExists[filePath]
-               w.mu.Unlock()
-               if !doesExist {
-                       // Send create event
-                       w.Events <- newCreateEvent(filePath)
-               }
+               err := w.sendFileCreatedEventIfNew(filePath, fileInfo)
 
-               // like watchDirectoryFiles (but without doing another ReadDir)
-               filePath, err = w.internalWatch(filePath, fileInfo)
                if err != nil {
                        return
                }
+       }
+}
 
-               w.mu.Lock()
-               w.fileExists[filePath] = true
-               w.mu.Unlock()
+// sendFileCreatedEvent sends a create event if the file isn't already being tracked.
+func (w *Watcher) sendFileCreatedEventIfNew(filePath string, fileInfo os.FileInfo) (err error) {
+       w.mu.Lock()
+       _, doesExist := w.fileExists[filePath]
+       w.mu.Unlock()
+       if !doesExist {
+               // Send create event
+               w.Events <- newCreateEvent(filePath)
+       }
+
+       // like watchDirectoryFiles (but without doing another ReadDir)
+       filePath, err = w.internalWatch(filePath, fileInfo)
+       if err != nil {
+               return err
        }
+
+       w.mu.Lock()
+       w.fileExists[filePath] = true
+       w.mu.Unlock()
+
+       return nil
 }
 
 func (w *Watcher) internalWatch(name string, fileInfo os.FileInfo) (string, error) {