Why won't this conversion-rate server compile, and is its RWMutex discipline correct?
This server refreshes currency rates once a minute in a background goroutine and serves them over HTTP. It does not compile. Also judge whether its RWMutex discipline is correct.
func main() {
mu &sync.RWMutex{}
rates, _ := readConversionRates()
go func() {
for {
time.Sleep(time.Minute)
tmp, err := readConversionRates()
if err != nil {
continue
}
mu.Lock()
rates = tmp
mu.Unlock()
}
}()
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
mu.RLock()
defer mu.RUnlock()
rate := rates["RUB"]
fmt.Fprint(w, 140.0/rate)
})
if err := http.ListenAndServe(":8080", nil); err != http.ErrServerClosed {
log.Fatal(err)
}
}Find and fix the bug.
mu &sync.RWMutex{} is missing := — write mu := &sync.RWMutex{}. The lock discipline is otherwise sound: the updater reassigns rates under Lock, handlers read it under RLock, so every access is guarded. Keep the != http.ErrServerClosed check on ListenAndServe.
- ✗Assuming the missing
:=is a typo for=—muis undeclared, so only:=compiles - ✗Believing reassigning the map under Lock races with RLock readers — the lock serializes both
- ✗Treating a clean
ListenAndServeshutdown as an error — it returnshttp.ErrServerClosed
- →Why is reassigning the
ratesmap underLocksafe, but mutating it underRLockwould not be? - →What goes wrong if the background updater used
RLockinstead ofLockto swaprates?
Conversion-rate server with a background updater
package main
import (
"fmt"
"log"
"net/http"
"sync"
"time"
)
func main() {
mu &sync.RWMutex{} // FIXME: does not compile
rates, err := readConversionRates()
if err != nil {
log.Fatalf("read initial conversion rates: %s", err)
}
// background task: refresh rates once a minute
go func() {
for {
time.Sleep(time.Minute)
tmp, err := readConversionRates()
if err != nil {
log.Printf("ERR: update conversion rates: %s", err)
continue
}
mu.Lock()
rates = tmp // reassign the whole map under Lock
mu.Unlock()
}
}()
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
from, val := "RUB", 140.0
mu.RLock()
defer mu.RUnlock()
rate, ok := rates[from] // read under RLock
if !ok {
http.NotFound(w, r)
return
}
fmt.Fprint(w, val/rate)
})
if err := http.ListenAndServe(":8080", nil); err != http.ErrServerClosed {
log.Fatal(err)
}
}
func readConversionRates() (map[string]float64, error) {
time.Sleep(100 * time.Millisecond) // simulate slow I/O
return map[string]float64{"USD": 1.0, "RUB": 70.0}, nil
}The only compile error is the line mu &sync.RWMutex{}: it is missing the short-declaration operator :=. The fix is:
mu := &sync.RWMutex{}The lock discipline is correct as written. The background updater reassigns the rates variable wholesale under Lock, and handlers read it under RLock. Because both the write and the reads go through the same mu, there is no race: Lock excludes every RLock, and the happens-before edge Unlock → RLock makes the new map visible to readers.
The key point is why reassignment works here while in-place mutation would not: the variable rates is mutex-protected, so swapping the pointer to a fresh map is safe. If the updater instead wrote rates[k] = v under RLock, that would be a write to the map under a read lock — a genuine data race.
The if err := http.ListenAndServe(...); err != http.ErrServerClosed check is also correct: ListenAndServe always returns a non-nil error, and on graceful Shutdown it returns exactly http.ErrServerClosed, which should not be treated as fatal.
