3 files changed, 92 insertions, 76 deletions
diff --git a/src/pkg/expvar/Makefile b/src/pkg/expvar/Makefile
deleted file mode 100644
index 5619630d1..000000000
--- a/src/pkg/expvar/Makefile
+++ /dev/null
@@ -1,11 +0,0 @@
-# Copyright 2009 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.
-
-include ../../Make.inc
-
-TARG=expvar
-GOFILES=\
-	expvar.go\
-
-include ../../Make.pkg
diff --git a/src/pkg/expvar/expvar.go b/src/pkg/expvar/expvar.go
index 7b733faf6..b06599505 100644
--- a/src/pkg/expvar/expvar.go
+++ b/src/pkg/expvar/expvar.go
@@ -16,17 +16,17 @@
 //
 // The package is sometimes only imported for the side effect of
 // registering its HTTP handler and the above variables.  To use it
-// this way, simply link this package into your program:
+// this way, link this package into your program:
 //	import _ "expvar"
 //
 package expvar
 
 import (
 	"bytes"
+	"encoding/json"
 	"fmt"
-	"http"
-	"json"
 	"log"
+	"net/http"
 	"os"
 	"runtime"
 	"strconv"
@@ -41,10 +41,14 @@ type Var interface {
 // Int is a 64-bit integer variable that satisfies the Var interface.
 type Int struct {
 	i  int64
-	mu sync.Mutex
+	mu sync.RWMutex
 }
 
-func (v *Int) String() string { return strconv.Itoa64(v.i) }
+func (v *Int) String() string {
+	v.mu.RLock()
+	defer v.mu.RUnlock()
+	return strconv.FormatInt(v.i, 10)
+}
 
 func (v *Int) Add(delta int64) {
 	v.mu.Lock()
@@ -61,10 +65,14 @@ func (v *Int) Set(value int64) {
 // Float is a 64-bit float variable that satisfies the Var interface.
 type Float struct {
 	f  float64
-	mu sync.Mutex
+	mu sync.RWMutex
 }
 
-func (v *Float) String() string { return strconv.Ftoa64(v.f, 'g', -1) }
+func (v *Float) String() string {
+	v.mu.RLock()
+	defer v.mu.RUnlock()
+	return strconv.FormatFloat(v.f, 'g', -1, 64)
+}
 
 // Add adds delta to v.
 func (v *Float) Add(delta float64) {
@@ -83,7 +91,7 @@ func (v *Float) Set(value float64) {
 // Map is a string-to-Var map variable that satisfies the Var interface.
 type Map struct {
 	m  map[string]Var
-	mu sync.Mutex
+	mu sync.RWMutex
 }
 
 // KeyValue represents a single entry in a Map.
@@ -93,19 +101,19 @@ type KeyValue struct {
 }
 
 func (v *Map) String() string {
-	v.mu.Lock()
-	defer v.mu.Unlock()
-	b := new(bytes.Buffer)
-	fmt.Fprintf(b, "{")
+	v.mu.RLock()
+	defer v.mu.RUnlock()
+	var b bytes.Buffer
+	fmt.Fprintf(&b, "{")
 	first := true
 	for key, val := range v.m {
 		if !first {
-			fmt.Fprintf(b, ", ")
+			fmt.Fprintf(&b, ", ")
 		}
-		fmt.Fprintf(b, "\"%s\": %v", key, val.String())
+		fmt.Fprintf(&b, "\"%s\": %v", key, val)
 		first = false
 	}
-	fmt.Fprintf(b, "}")
+	fmt.Fprintf(&b, "}")
 	return b.String()
 }
 
@@ -115,8 +123,8 @@ func (v *Map) Init() *Map {
 }
 
 func (v *Map) Get(key string) Var {
-	v.mu.Lock()
-	defer v.mu.Unlock()
+	v.mu.RLock()
+	defer v.mu.RUnlock()
 	return v.m[key]
 }
 
@@ -127,12 +135,17 @@ func (v *Map) Set(key string, av Var) {
 }
 
 func (v *Map) Add(key string, delta int64) {
-	v.mu.Lock()
-	defer v.mu.Unlock()
+	v.mu.RLock()
 	av, ok := v.m[key]
+	v.mu.RUnlock()
 	if !ok {
-		av = new(Int)
-		v.m[key] = av
+		// check again under the write lock
+		v.mu.Lock()
+		if _, ok = v.m[key]; !ok {
+			av = new(Int)
+			v.m[key] = av
+		}
+		v.mu.Unlock()
 	}
 
 	// Add to Int; ignore otherwise.
@@ -143,12 +156,17 @@ func (v *Map) Add(key string, delta int64) {
 
 // AddFloat adds delta to the *Float value stored under the given map key.
 func (v *Map) AddFloat(key string, delta float64) {
-	v.mu.Lock()
-	defer v.mu.Unlock()
+	v.mu.RLock()
 	av, ok := v.m[key]
+	v.mu.RUnlock()
 	if !ok {
-		av = new(Float)
-		v.m[key] = av
+		// check again under the write lock
+		v.mu.Lock()
+		if _, ok = v.m[key]; !ok {
+			av = new(Float)
+			v.m[key] = av
+		}
+		v.mu.Unlock()
 	}
 
 	// Add to Float; ignore otherwise.
@@ -157,28 +175,34 @@ func (v *Map) AddFloat(key string, delta float64) {
 	}
 }
 
-// TODO(rsc): Make sure map access in separate thread is safe.
-func (v *Map) iterate(c chan<- KeyValue) {
+// Do calls f for each entry in the map.
+// The map is locked during the iteration,
+// but existing entries may be concurrently updated.
+func (v *Map) Do(f func(KeyValue)) {
+	v.mu.RLock()
+	defer v.mu.RUnlock()
 	for k, v := range v.m {
-		c <- KeyValue{k, v}
+		f(KeyValue{k, v})
 	}
-	close(c)
-}
-
-func (v *Map) Iter() <-chan KeyValue {
-	c := make(chan KeyValue)
-	go v.iterate(c)
-	return c
 }
 
 // String is a string variable, and satisfies the Var interface.
 type String struct {
-	s string
+	s  string
+	mu sync.RWMutex
 }
 
-func (v *String) String() string { return strconv.Quote(v.s) }
+func (v *String) String() string {
+	v.mu.RLock()
+	defer v.mu.RUnlock()
+	return strconv.Quote(v.s)
+}
 
-func (v *String) Set(value string) { v.s = value }
+func (v *String) Set(value string) {
+	v.mu.Lock()
+	defer v.mu.Unlock()
+	v.s = value
+}
 
 // Func implements Var by calling the function
 // and formatting the returned value using JSON.
@@ -190,10 +214,12 @@ func (f Func) String() string {
 }
 
 // All published variables.
-var vars map[string]Var = make(map[string]Var)
-var mutex sync.Mutex
+var (
+	mutex sync.RWMutex
+	vars  map[string]Var = make(map[string]Var)
+)
 
-// Publish declares an named exported variable. This should be called from a
+// Publish declares a named exported variable. This should be called from a
 // package's init function when it creates its Vars. If the name is already
 // registered then this will log.Panic.
 func Publish(name string, v Var) {
@@ -207,17 +233,11 @@ func Publish(name string, v Var) {
 
 // Get retrieves a named exported variable.
 func Get(name string) Var {
+	mutex.RLock()
+	defer mutex.RUnlock()
 	return vars[name]
 }
 
-// RemoveAll removes all exported variables.
-// This is for tests; don't call this on a real server.
-func RemoveAll() {
-	mutex.Lock()
-	defer mutex.Unlock()
-	vars = make(map[string]Var)
-}
-
 // Convenience functions for creating new exported variables.
 
 func NewInt(name string) *Int {
@@ -244,31 +264,28 @@ func NewString(name string) *String {
 	return v
 }
 
-// TODO(rsc): Make sure map access in separate thread is safe.
-func iterate(c chan<- KeyValue) {
+// Do calls f for each exported variable.
+// The global variable map is locked during the iteration,
+// but existing entries may be concurrently updated.
+func Do(f func(KeyValue)) {
+	mutex.RLock()
+	defer mutex.RUnlock()
 	for k, v := range vars {
-		c <- KeyValue{k, v}
+		f(KeyValue{k, v})
 	}
-	close(c)
-}
-
-func Iter() <-chan KeyValue {
-	c := make(chan KeyValue)
-	go iterate(c)
-	return c
 }
 
 func expvarHandler(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")
 	fmt.Fprintf(w, "{\n")
 	first := true
-	for name, value := range vars {
+	Do(func(kv KeyValue) {
 		if !first {
 			fmt.Fprintf(w, ",\n")
 		}
 		first = false
-		fmt.Fprintf(w, "%q: %s", name, value)
-	}
+		fmt.Fprintf(w, "%q: %s", kv.Key, kv.Value)
+	})
 	fmt.Fprintf(w, "\n}\n")
 }
 
@@ -277,11 +294,13 @@ func cmdline() interface{} {
 }
 
 func memstats() interface{} {
-	return runtime.MemStats
+	stats := new(runtime.MemStats)
+	runtime.ReadMemStats(stats)
+	return *stats
 }
 
 func init() {
-	http.Handle("/debug/vars", http.HandlerFunc(expvarHandler))
+	http.HandleFunc("/debug/vars", expvarHandler)
 	Publish("cmdline", Func(cmdline))
 	Publish("memstats", Func(memstats))
 }
diff --git a/src/pkg/expvar/expvar_test.go b/src/pkg/expvar/expvar_test.go
index 8f7a48168..bbd9dd8d6 100644
--- a/src/pkg/expvar/expvar_test.go
+++ b/src/pkg/expvar/expvar_test.go
@@ -5,10 +5,18 @@
 package expvar
 
 import (
-	"json"
+	"encoding/json"
 	"testing"
 )
 
+// RemoveAll removes all exported variables.
+// This is for tests only.
+func RemoveAll() {
+	mutex.Lock()
+	defer mutex.Unlock()
+	vars = make(map[string]Var)
+}
+
 func TestInt(t *testing.T) {
 	reqs := NewInt("requests")
 	if reqs.i != 0 {