Merge pull request #221 from prometheus/fix/race-conditions

Fix/document race conditions

README.md

@@ -107,6 +107,22 @@ architecture and release identification remarks for us.
 
     $ make test
 
+### Race Detector
+
+Go 1.1 includes a [race
+detector](http://tip.golang.org/doc/articles/race_detector.html) which can be
+enabled at build time. Here's how to use it with prometheus (assumes that
+you've already run a successful build).
+
+To run the tests with race detection:
+
+    $ GORACE="log_path=/tmp/foo" go test -race ./...
+
+To run the server with race detection:
+
+    $ go build -race .
+    $ GORACE="log_path=/tmp/foo" ./prometheus
+
 [![Build Status](https://travis-ci.org/prometheus/prometheus.png)](https://travis-ci.org/prometheus/prometheus)
 
 ## Contributing

utility/uncertaintygroup.go

@@ -15,6 +15,7 @@ package utility
 
 import (
 	"fmt"
+	"sync"
 )
 
 type state int
@@ -62,18 +63,19 @@ type uncertaintyGroup struct {
 	successes uint
 	results   chan error
 	anomalies []error
+	sync.Mutex
 }
 
-func (g uncertaintyGroup) Succeed() {
-	if g.state == finished {
+func (g *uncertaintyGroup) Succeed() {
+	if g.isFinished() {
 		panic("cannot remark when done")
 	}
 
 	g.results <- nil
 }
 
-func (g uncertaintyGroup) Fail(err error) {
-	if g.state == finished {
+func (g *uncertaintyGroup) Fail(err error) {
+	if g.isFinished() {
 		panic("cannot remark when done")
 	}
 
@@ -84,22 +86,42 @@ func (g uncertaintyGroup) Fail(err error) {
 	g.results <- err
 }
 
-func (g uncertaintyGroup) MayFail(err error) {
-	if g.state == finished {
+func (g *uncertaintyGroup) MayFail(err error) {
+	if g.isFinished() {
 		panic("cannot remark when done")
 	}
 
 	g.results <- err
 }
 
-func (g *uncertaintyGroup) Wait() bool {
+func (g *uncertaintyGroup) isFinished() bool {
+	g.Lock()
+	defer g.Unlock()
+
+	return g.state == finished
+}
+
+func (g *uncertaintyGroup) finish() {
+	g.Lock()
+	defer g.Unlock()
+
+	g.state = finished
+}
+
+func (g *uncertaintyGroup) start() {
+	g.Lock()
+	defer g.Unlock()
+
 	if g.state != unstarted {
 		panic("cannot restart")
 	}
 
-	defer close(g.results)
-
 	g.state = started
+}
+
+func (g *uncertaintyGroup) Wait() bool {
+	defer close(g.results)
+	g.start()
 
 	for g.remaining > 0 {
 		result := <-g.results
@@ -113,12 +135,12 @@ func (g *uncertaintyGroup) Wait() bool {
 		g.remaining--
 	}
 
-	g.state = finished
+	g.finish()
 
 	return len(g.anomalies) == 0
 }
 
-func (g uncertaintyGroup) Errors() []error {
+func (g *uncertaintyGroup) Errors() []error {
 	if g.state != finished {
 		panic("cannot provide errors until finished")
 	}
@@ -126,7 +148,7 @@ func (g uncertaintyGroup) Errors() []error {
 	return g.anomalies
 }
 
-func (g uncertaintyGroup) String() string {
+func (g *uncertaintyGroup) String() string {
 	return fmt.Sprintf("UncertaintyGroup %s with %s failures", g.state, g.anomalies)
 }
 

web/status.go

@@ -48,10 +48,11 @@ func (h *StatusHandler) ServeRequestsForever() {
 	})
 
 	h.PrometheusStatus = &PrometheusStatus{
-		BuildInfo:   h.BuildInfo,
-		Config:      h.Config.String(),
-		Flags:       flags,
-		Rules:       "TODO: list rules here",
+		BuildInfo: h.BuildInfo,
+		Config:    h.Config.String(),
+		Flags:     flags,
+		Rules:     "TODO: list rules here",
+		// BUG: race condition, concurrent map access
 		TargetPools: h.TargetManager.Pools(),
 	}
 

web/web.go

@@ -41,6 +41,10 @@ type WebService struct {
 func (w WebService) ServeForever() error {
 	gorest.RegisterService(w.MetricsHandler)
 
+	exp.Handle("/favicon.ico", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Error(w, "", 404)
+	}))
+
 	// TODO(julius): This will need to be rewritten once the exp package provides
 	// the coarse mux behaviors via a wrapper function.
 	exp.Handle("/debug/pprof/", http.HandlerFunc(pprof.Index))