Fix a couple of races

Fixes #16

defaultexclude.go

@@ -20,7 +20,7 @@ var defaultExcludeMatcher multiMatcher
 
 func init() {
 	for _, pattern := range defaultExcludes {
-		m := &regexMatcher{regex: regexp.MustCompile(pattern), inverse: true}
+		m := newRegexMatcher(regexp.MustCompile(pattern), true)
 		defaultExcludeMatcher = append(defaultExcludeMatcher, m)
 	}
 }

main.go

@@ -97,7 +97,7 @@ func cleanup(reason string) {
 	fmt.Println(reason)
 	wg := &sync.WaitGroup{}
 	for _, reflex := range reflexes {
-		if reflex.done != nil {
+		if reflex.Running() {
 			wg.Add(1)
 			go func(reflex *Reflex) {
 				reflex.terminate()

match.go

@@ -6,6 +6,7 @@ import (
 	"regexp"
 	"regexp/syntax"
 	"strings"
+	"sync"
 )
 
 // A Matcher decides whether some filename matches its set of patterns.
@@ -30,17 +31,14 @@ func ParseMatchers(regexes, inverseRegexes, globs, inverseGlobs []string) (m Mat
 		if err != nil {
 			return nil, err
 		}
-		matchers = append(matchers, &regexMatcher{regex: regex})
+		matchers = append(matchers, newRegexMatcher(regex, false))
 	}
 	for _, r := range inverseRegexes {
 		regex, err := regexp.Compile(r)
 		if err != nil {
 			return nil, err
 		}
-		matchers = append(matchers, &regexMatcher{
-			regex:   regex,
-			inverse: true,
-		})
+		matchers = append(matchers, newRegexMatcher(regex, true))
 	}
 	for _, g := range globs {
 		matchers = append(matchers, &globMatcher{glob: g})
@@ -88,14 +86,23 @@ type regexMatcher struct {
 	regex   *regexp.Regexp
 	inverse bool
 
-	canExcludePrefix bool // This regex has no $, \z, or \b -- see ExcludePrefix
+	mu               *sync.Mutex // protects following
+	canExcludePrefix bool        // This regex has no $, \z, or \b -- see ExcludePrefix
 	excludeChecked   bool
 }
 
 func (m *regexMatcher) Match(name string) bool {
 	return m.regex.MatchString(name) != m.inverse
 }
 
+func newRegexMatcher(regex *regexp.Regexp, inverse bool) *regexMatcher {
+	return &regexMatcher{
+		regex:   regex,
+		inverse: inverse,
+		mu:      new(sync.Mutex),
+	}
+}
+
 // ExcludePrefix returns whether this matcher cannot possibly match any path with a particular prefix.
 // The question is: given a regex r and some prefix p which r accepts, is there any string s that has p as a
 // prefix that r does not accept?
@@ -120,6 +127,8 @@ func (m *regexMatcher) ExcludePrefix(prefix string) bool {
 		return false
 	}
 
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	if !m.excludeChecked {
 		r, err := syntax.Parse(m.regex.String(), syntax.Perl)
 		if err != nil {

match_test.go

@@ -17,18 +17,18 @@ func TestGlobMatcher(t *testing.T) {
 }
 
 func TestRegexMatcher(t *testing.T) {
-	m := &regexMatcher{regex: regexp.MustCompile("foo.*")}
+	m := newRegexMatcher(regexp.MustCompile("foo.*"), false)
 	equals(t, true, m.Match("foo"))
 	equals(t, true, m.Match("foobar"))
 	equals(t, false, m.Match("bar"))
-	m = &regexMatcher{regex: regexp.MustCompile("foo.*"), inverse: true}
+	m = newRegexMatcher(regexp.MustCompile("foo.*"), true)
 	equals(t, false, m.Match("foo"))
 	equals(t, false, m.Match("foobar"))
 	equals(t, true, m.Match("bar"))
 }
 
 func TestExcludePrefix(t *testing.T) {
-	m := &regexMatcher{regex: regexp.MustCompile("foo")}
+	m := newRegexMatcher(regexp.MustCompile("foo"), false)
 	equals(t, false, m.ExcludePrefix("bar")) // Never true for non-inverted
 
 	for _, testCase := range []struct {
@@ -42,16 +42,16 @@ func TestExcludePrefix(t *testing.T) {
 		{re: `foo\b`, prefix: "foo", expected: false},
 		{re: `(foo\b)|(baz$)`, prefix: "foo", expected: false},
 	} {
-		m := &regexMatcher{regex: regexp.MustCompile(testCase.re), inverse: true}
+		m := newRegexMatcher(regexp.MustCompile(testCase.re), true)
 		equals(t, testCase.expected, m.ExcludePrefix(testCase.prefix))
 	}
 }
 
 func TestMultiMatcher(t *testing.T) {
 	m := multiMatcher{
-		&regexMatcher{regex: regexp.MustCompile("foo")},
-		&regexMatcher{regex: regexp.MustCompile(`\.go$`)},
-		&regexMatcher{regex: regexp.MustCompile("foobar"), inverse: true},
+		newRegexMatcher(regexp.MustCompile("foo"), false),
+		newRegexMatcher(regexp.MustCompile(`\.go$`), false),
+		newRegexMatcher(regexp.MustCompile("foobar"), true),
 	}
 	equals(t, true, m.Match("foo.go"))
 	equals(t, true, m.Match("foo/bar.go"))

reflex.go

@@ -28,11 +28,13 @@ type Reflex struct {
 	subSymbol    string
 	done         chan struct{}
 
+	mu      *sync.Mutex // protects killed and running
+	killed  bool
+	running bool
+
 	// Used for services (startService = true)
-	cmd    *exec.Cmd
-	tty    *os.File
-	mu     *sync.Mutex // protects killed
-	killed bool
+	cmd *exec.Cmd
+	tty *os.File
 }
 
 // NewReflex prepares a Reflex from a Config, with sanity checking.
@@ -84,6 +86,7 @@ func NewReflex(c *Config) (*Reflex, error) {
 		onlyDirs:     c.onlyDirs,
 		command:      c.command,
 		subSymbol:    c.subSymbol,
+		done:         make(chan struct{}),
 
 		mu: &sync.Mutex{},
 	}
@@ -173,7 +176,7 @@ func (r *Reflex) batch(out chan<- string, in <-chan string) {
 func (r *Reflex) runEach(names <-chan string) {
 	for name := range names {
 		if r.startService {
-			if r.done != nil {
+			if r.Running() {
 				infoPrintln(r.id, "Killing service")
 				r.terminate()
 			}
@@ -182,7 +185,9 @@ func (r *Reflex) runEach(names <-chan string) {
 		} else {
 			r.runCommand(name, stdout)
 			<-r.done
-			r.done = nil
+			r.mu.Lock()
+			r.running = false
+			r.mu.Unlock()
 		}
 	}
 }
@@ -212,7 +217,7 @@ func (r *Reflex) terminate() {
 			// the process may have created.
 			if err := syscall.Kill(-1*r.cmd.Process.Pid, sig); err != nil {
 				infoPrintln(r.id, "Error killing:", err)
-				if err.(syscall.Errno) == syscall.ESRCH { // "no such process"
+				if err.(syscall.Errno) == syscall.ESRCH { // no such process
 					return
 				}
 			}
@@ -261,17 +266,15 @@ func (r *Reflex) runCommand(name string, stdout chan<- OutMsg) {
 		// errors here unless I can find a better way to handle it.
 	}()
 
-	done := make(chan struct{})
-	r.done = done
+	r.mu.Lock()
+	r.running = true
+	r.mu.Unlock()
 	go func() {
 		err := cmd.Wait()
-		r.mu.Lock()
-		killed := r.killed
-		r.mu.Unlock()
-		if !killed && err != nil {
+		if !r.Killed() && err != nil {
 			stdout <- OutMsg{r.id, fmt.Sprintf("(error exit: %s)", err)}
 		}
-		done <- struct{}{}
+		r.done <- struct{}{}
 		if flagSequential {
 			seqCommands.Unlock()
 		}
@@ -290,3 +293,15 @@ func (r *Reflex) Start(changes <-chan string) {
 		r.runCommand("", stdout)
 	}
 }
+
+func (r *Reflex) Killed() bool {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	return r.killed
+}
+
+func (r *Reflex) Running() bool {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	return r.running
+}