Coordination Anti-Patterns — Refactoring Practice¶

func TestTransfer_NoDeadlock(t *testing.T) {
    a, b := &Account{ID: 1, Balance: 1_000_000}, &Account{ID: 2, Balance: 1_000_000}
    var wg sync.WaitGroup
    for i := 0; i < 1000; i++ {
        wg.Add(2)
        go func() { defer wg.Done(); Transfer(a, b, 1) }()
        go func() { defer wg.Done(); Transfer(b, a, 1) }() // reverse direction
    }
    done := make(chan struct{})
    go func() { wg.Wait(); close(done) }()
    select {
    case <-done:
    case <-time.After(2 * time.Second):
        t.Fatal("deadlock: transfers did not complete") // the Before version hangs here
    }
    if got := a.Balance + b.Balance; got != 2_000_000 {
        t.Fatalf("money not conserved: %d", got) // also catches lost updates
    }
}

// After — both directions acquire locks in the same (by-ID) order.
func Transfer(from, to *Account, amt int64) {
    if from.ID == to.ID {
        return // self-transfer is a no-op; never lock the same mutex twice
    }
    first, second := from, to
    if first.ID > second.ID {
        first, second = second, first // total order: lower ID first
    }
    first.mu.Lock()
    defer first.mu.Unlock()
    second.mu.Lock()
    defer second.mu.Unlock()

    from.Balance -= amt
    to.Balance += amt
}

func BenchmarkRefresh(b *testing.B) {
    c := &RateCache{rates: map[string]float64{}}
    b.RunParallel(func(pb *testing.PB) {
        for pb.Next() {
            c.Refresh("USD")
        }
    })
}
// Before: throughput ≈ 1 / 50ms regardless of GOMAXPROCS — the lock is the bottleneck.
// After:  the 50ms calls run concurrently; only the tiny map write serializes.

// After — the lock guards only the map mutation; the I/O is lock-free.
func (c *RateCache) Refresh(ccy string) (float64, error) {
    v, err := httpGetRate(ccy) // no lock held during the network call
    if err != nil {
        return 0, err
    }
    c.mu.Lock()
    c.rates[ccy] = v // critical section is now nanoseconds, not 200ms
    c.mu.Unlock()
    return v, nil
}

func BenchmarkInc(b *testing.B) {
    c := NewCounters()
    keys := []string{"a", "b", "c", "d", "e", "f", "g", "h"}
    b.RunParallel(func(pb *testing.PB) {
        i := 0
        for pb.Next() {
            c.Inc(keys[i%len(keys)]) // distinct keys, yet contended on one mutex
            i++
        }
    })
}
// Before: ns/op flat as you raise -cpu; lock contention dominates.
// After:  ns/op drops as work spreads across stripes.

// After — lock striping: independent keys hit independent locks.
const shards = 16 // power of two → mask instead of modulo

type shard struct {
    mu sync.Mutex
    m  map[string]int64
}

type Counters struct {
    s [shards]*shard
}

func NewCounters() *Counters {
    c := &Counters{}
    for i := range c.s {
        c.s[i] = &shard{m: make(map[string]int64)}
    }
    return c
}

func (c *Counters) shardFor(key string) *shard {
    h := fnv32(key)
    return c.s[h&(shards-1)] // mask works because shards is a power of two
}

func (c *Counters) Inc(key string) {
    s := c.shardFor(key)
    s.mu.Lock()
    s.m[key]++
    s.mu.Unlock()
}

func fnv32(s string) uint32 {
    h := uint32(2166136261)
    for i := 0; i < len(s); i++ {
        h = (h ^ uint32(s[i])) * 16777619
    }
    return h
}

func BenchmarkInc(b *testing.B) {
    var m Metric
    b.RunParallel(func(pb *testing.PB) {
        for pb.Next() {
            m.Inc()
        }
    })
}
// Before (mutex):  contended lock, more ns/op.
// After  (atomic): single LOCK-prefixed instruction, fewer ns/op.

// After — lock-free; the increment is a single atomic instruction.
type Metric struct {
    n atomic.Int64
}

func (m *Metric) Inc()        { m.n.Add(1) }
func (m *Metric) Get() int64  { return m.n.Load() }

// After — ordered locks, balances mutated under lock, I/O done lock-free.
func Transfer(from, to *Account, amt int64, audit AuditLog) error {
    if from.ID == to.ID {
        return nil
    }
    first, second := from, to
    if first.ID > second.ID {
        first, second = second, first
    }

    first.mu.Lock()
    second.mu.Lock()
    from.Balance -= amt
    to.Balance += amt
    // snapshot what the I/O needs, then drop the locks
    fromID, toID := from.ID, to.ID
    second.mu.Unlock()
    first.mu.Unlock()

    // I/O happens with NO locks held
    if err := audit.Write(fromID, toID, amt); err != nil {
        return fmt.Errorf("transfer committed but audit failed: %w", err)
    }
    return nil
}

// After — unchanged on purpose. The single mutex is the correct, simplest,
// fastest tool for a short critical section over a multi-field invariant.
// (Comment added documenting why atomics/striping/RWLock were rejected.)

// After — one owner goroutine; the channel provides ordering and safety.
type Ledger struct {
    applies chan int64
    reads   chan chan int64
}

func NewLedger() *Ledger {
    l := &Ledger{
        applies: make(chan int64),
        reads:   make(chan chan int64),
    }
    go l.loop()
    return l
}

func (l *Ledger) loop() {
    var balance int64 // owned solely by this goroutine — no lock, no race
    for {
        select {
        case d := <-l.applies:
            balance += d
        case reply := <-l.reads:
            reply <- balance
        }
    }
}

func (l *Ledger) Apply(delta int64) { l.applies <- delta }

func (l *Ledger) Balance() int64 {
    reply := make(chan int64)
    l.reads <- reply
    return <-reply
}

// After — sharded locks, load-outside-lock, single-flight dedup.
import "golang.org/x/sync/singleflight"

const shards = 16

type cacheShard struct {
    mu   sync.RWMutex
    data map[string]string
}

type Cache struct {
    shards [shards]*cacheShard
    group  singleflight.Group
    load   func(key string) (string, error)
}

func NewCache(load func(string) (string, error)) *Cache {
    c := &Cache{load: load}
    for i := range c.shards {
        c.shards[i] = &cacheShard{data: make(map[string]string)}
    }
    return c
}

func (c *Cache) shardFor(key string) *cacheShard { return c.shards[fnv32(key)&(shards-1)] }

func (c *Cache) Get(key string) (string, error) {
    s := c.shardFor(key)

    // fast path: read lock only
    s.mu.RLock()
    if v, ok := s.data[key]; ok {
        s.mu.RUnlock()
        return v, nil
    }
    s.mu.RUnlock()

    // slow path: load WITHOUT holding the shard lock, deduped per key
    v, err, _ := c.group.Do(key, func() (interface{}, error) {
        // re-check: another goroutine may have filled it while we waited
        s.mu.RLock()
        if got, ok := s.data[key]; ok {
            s.mu.RUnlock()
            return got, nil
        }
        s.mu.RUnlock()

        loaded, err := c.load(key) // no lock held during I/O
        if err != nil {
            return "", err
        }
        s.mu.Lock()
        s.data[key] = loaded
        s.mu.Unlock()
        return loaded, nil
    })
    if err != nil {
        return "", err
    }
    return v.(string), nil
}

// After — resource hierarchy: always lock the lower-indexed fork first.
type Fork struct {
    mu sync.Mutex
    id int
}

func philosopher(left, right *Fork) {
    first, second := left, right
    if first.id > second.id {
        first, second = second, first // total order over forks
    }
    for {
        first.mu.Lock()
        second.mu.Lock()
        eat()
        second.mu.Unlock()
        first.mu.Unlock()
    }
}

func BenchmarkGet(b *testing.B) {
    h := NewHolder(Config{MaxConns: 100})
    b.RunParallel(func(pb *testing.PB) {
        for pb.Next() {
            _ = h.Get()
        }
    })
}
// Mutex:   reads serialize.
// RWLock:  reads share, but still touch the lock's reader-count (cache contention).
// Atomic:  reads are a single pointer load — best read scalability.

// After — lock-free reads via atomic pointer swap (copy-on-write).
type Holder struct {
    ptr atomic.Pointer[Config]
}

func NewHolder(c Config) *Holder {
    h := &Holder{}
    h.ptr.Store(&c)
    return h
}

// Get is a single atomic load — no lock on the hot path.
func (h *Holder) Get() *Config { return h.ptr.Load() }

// Set publishes a new immutable snapshot; readers see all-or-nothing.
func (h *Holder) Set(update func(old Config) Config) {
    for {
        old := h.ptr.Load()
        next := update(*old) // build a fresh value off a copy
        if h.ptr.CompareAndSwap(old, &next) {
            return // lost updates impossible: CAS retries on concurrent writers
        }
    }
}