diff --git a/validator.go b/validator.go index 1b0492369..a54043be8 100644 --- a/validator.go +++ b/validator.go @@ -59,8 +59,7 @@ func (um *UserMap) LoadAuthenticatedEmailsFile() { atomic.StorePointer(&um.m, unsafe.Pointer(&updated)) } -func newValidatorImpl(domains []string, usersFile string, - done <-chan bool, onUpdate func()) func(string) bool { +func newValidatorImpl(domains []string, usersFile string, done <-chan bool, onUpdate func()) func(string) bool { validUsers := NewUserMap(usersFile, done, onUpdate) var allowAll bool diff --git a/validator_test.go b/validator_test.go index f91f41ce6..a8b5f7dee 100644 --- a/validator_test.go +++ b/validator_test.go @@ -20,7 +20,8 @@ func NewValidatorTest(t *testing.T) *ValidatorTest { if err != nil { t.Fatal("failed to create temp file: " + err.Error()) } - vt.done = make(chan bool, 1) + t.Logf("%s using temp file %s", t.Name(), vt.auth_email_file.Name()) + vt.done = make(chan bool) return vt } @@ -29,20 +30,17 @@ func (vt *ValidatorTest) TearDown() { os.Remove(vt.auth_email_file.Name()) } -func (vt *ValidatorTest) NewValidator(domains []string, - updated chan<- bool) func(string) bool { - return newValidatorImpl(domains, vt.auth_email_file.Name(), - vt.done, func() { - if vt.update_seen == false { - updated <- true - vt.update_seen = true - } - }) +func (vt *ValidatorTest) NewValidator(domains []string, updated chan<- bool) func(string) bool { + return newValidatorImpl(domains, vt.auth_email_file.Name(), vt.done, func() { + if vt.update_seen == false { + updated <- true + vt.update_seen = true + } + }) } // This will close vt.auth_email_file. func (vt *ValidatorTest) WriteEmails(t *testing.T, emails []string) { - defer vt.auth_email_file.Close() vt.auth_email_file.WriteString(strings.Join(emails, "\n")) if err := vt.auth_email_file.Close(); err != nil { t.Fatal("failed to close temp file " + diff --git a/validator_watcher_copy_test.go b/validator_watcher_copy_test.go index 68c4cb7d3..11bbdb891 100644 --- a/validator_watcher_copy_test.go +++ b/validator_watcher_copy_test.go @@ -1,4 +1,4 @@ -// +build go1.3,!plan9,!solaris,!windows +// +build !plan9,!solaris,!windows // Turns out you can't copy over an existing file on Windows. @@ -32,7 +32,7 @@ func TestValidatorOverwriteEmailListViaCopyingOver(t *testing.T) { vt.WriteEmails(t, []string{"xyzzy@example.com"}) domains := []string(nil) - updated := make(chan bool) + updated := make(chan bool, 1) validator := vt.NewValidator(domains, updated) if !validator("xyzzy@example.com") { diff --git a/validator_watcher_test.go b/validator_watcher_test.go index dc16a7da3..a8edbcbaa 100644 --- a/validator_watcher_test.go +++ b/validator_watcher_test.go @@ -1,4 +1,4 @@ -// +build go1.3,!plan9,!solaris +// +build !plan9,!solaris package main @@ -49,7 +49,7 @@ func TestValidatorOverwriteEmailListDirectly(t *testing.T) { "plugh@example.com", }) domains := []string(nil) - updated := make(chan bool) + updated := make(chan bool, 1) validator := vt.NewValidator(domains, updated) if !validator("xyzzy@example.com") { diff --git a/watcher.go b/watcher.go index b739d1421..a32e72e2a 100644 --- a/watcher.go +++ b/watcher.go @@ -1,4 +1,4 @@ -// +build go1.3,!plan9,!solaris +// +build !plan9,!solaris package main @@ -11,23 +11,18 @@ import ( "github.com/fsnotify/fsnotify" ) -func WaitForReplacement(filename string, op fsnotify.Op, - watcher *fsnotify.Watcher) { - const sleep_interval = 50 * time.Millisecond +func WaitForReplacement(filename string, watcher *fsnotify.Watcher) { + for i := 0; i < 20; i++ { + time.Sleep(100 * time.Millisecond) - // Avoid a race when fsnofity.Remove is preceded by fsnotify.Chmod. - if op&fsnotify.Chmod != 0 { - time.Sleep(sleep_interval) - } - for { if _, err := os.Stat(filename); err == nil { if err := watcher.Add(filename); err == nil { log.Printf("watching resumed for %s", filename) return } } - time.Sleep(sleep_interval) } + log.Printf("failed to resume watching for %s", filename) } func WatchForUpdates(filename string, done <-chan bool, action func()) { @@ -37,22 +32,22 @@ func WatchForUpdates(filename string, done <-chan bool, action func()) { log.Fatal("failed to create watcher for ", filename, ": ", err) } go func() { - defer watcher.Close() for { select { case _ = <-done: log.Printf("Shutting down watcher for: %s", filename) - break + watcher.Close() + return case event := <-watcher.Events: // On Arch Linux, it appears Chmod events precede Remove events, // which causes a race between action() and the coming Remove event. - // If the Remove wins, the action() (which calls - // UserMap.LoadAuthenticatedEmailsFile()) crashes when the file - // can't be opened. - if event.Op&(fsnotify.Remove|fsnotify.Rename|fsnotify.Chmod) != 0 { + if event.Op == fsnotify.Chmod { + continue + } + if event.Op&(fsnotify.Remove|fsnotify.Rename) != 0 { log.Printf("watching interrupted on event: %s", event) watcher.Remove(filename) - WaitForReplacement(filename, event.Op, watcher) + WaitForReplacement(filename, watcher) } log.Printf("reloading after event: %s", event) action() diff --git a/watcher_unsupported.go b/watcher_unsupported.go index 937f49bbc..117bdb2d4 100644 --- a/watcher_unsupported.go +++ b/watcher_unsupported.go @@ -1,4 +1,4 @@ -// +build !go1.3 plan9 solaris +// +build plan9 solaris package main