Synchronization Misuse — Refactoring Practice¶

Category: Concurrency Anti-Patterns → Synchronization Misuse — locks and memory primitives applied wrongly, so "synchronized" code still races. Covers (collectively): Double-Checked Locking · Volatile Misuse / Wrong Memory Ordering · Race-Prone Lazy Init

These are not "spot the race" puzzles — find-bug.md does that. Here each "Before" is working-but-unsafe (or unsafe only under load): it passes a single-threaded test, ships, and then corrupts state once in 10,000 production runs. Your job is to transform it into something provably race-free — and, where the original over-synchronized, faster — without changing its observable contract.

The skill on display is the process, not just the destination. Concurrency bugs do not yield to "read it harder." They yield to a loop:

1. Reproduce with a tool. A race that "can't happen" happens under go test -race, java -jar jcstress, or a stress harness that hammers the code from N threads. If you can't make it fail, you can't prove you fixed it.
2. Fix with the smallest correct primitive. sync.Once for one-time init; a lock for a compound action; an atomic only for a genuinely independent single operation. Reach for the weakest tool that is still correct.
3. Verify race-free. Re-run the detector and the stress harness. A green race detector over many iterations is your characterization test for "no data race." Where you also claimed a speedup, attach a benchmark.

A word on Python. CPython's GIL serializes bytecode, so a single x += 1 rarely tears — but it is not atomic (it's LOAD/ADD/STORE, interruptible between ops), and compound actions (if k not in d: d[k] = ...) still race. The GIL hides data races, it does not remove the logic races. Free-threaded builds (PEP 703, 3.13+ --disable-gil) remove even that cover. So the Python examples below show the logic race that survives the GIL.

How to use this file: read the "Before", write down your fix and how you'd reproduce the race before expanding the solution, then compare. The gap between your plan and the worked plan is the learning.

Table of Contents¶

Exercise 1 — Race-prone singleton → sync.Once¶

Anti-pattern: Race-Prone Lazy Init. Goal: make first-call initialization safe under concurrent callers. Constraints: preserve the API (Instance() *Config); exactly one Config must ever be built; preserve laziness (no init until first call).

// Before — two goroutines can both see cfg == nil and both build one.
var cfg *Config

func Instance() *Config {
    if cfg == nil {
        cfg = loadConfig() // expensive: reads file + parses
    }
    return cfg
}

func TestInstance_Race(t *testing.T) {
 var wg sync.WaitGroup
 got := make([]*Config, 50)
 for i := 0; i < 50; i++ {
     wg.Add(1)
     go func(i int) { defer wg.Done(); got[i] = Instance() }(i)
 }
 wg.Wait()
 for _, g := range got {
     if g != got[0] {
         t.Fatalf("got multiple instances")
     }
 }
}

// After — sync.Once: runs once, publishes safely, stays lazy.
var (
    cfg     *Config
    cfgOnce sync.Once
)

func Instance() *Config {
    cfgOnce.Do(func() { cfg = loadConfig() })
    return cfg
}

Exercise 2 — Lazy field → initialization-on-demand holder¶

Anti-pattern: Race-Prone Lazy Init. Goal: lazily initialize an expensive field, thread-safely, with no locking on the hot path. Constraints: Java; preserve laziness; getCache() returns the same instance for all callers.

// Before — classic racy lazy init; two threads build two caches.
class Registry {
    private ExpensiveCache cache; // not volatile, not guarded

    ExpensiveCache getCache() {
        if (cache == null) {
            cache = new ExpensiveCache(); // visible half-built to other threads
        }
        return cache;
    }
}

// After — holder idiom: JVM class-init gives lazy, safe, lock-free publication.
class Registry {
    private static class Holder {
        static final ExpensiveCache INSTANCE = new ExpensiveCache();
    }

    ExpensiveCache getCache() {
        return Holder.INSTANCE;
    }
}

Exercise 3 — Hand-rolled DCL → correct volatile DCL¶

Anti-pattern: Double-Checked Locking + Volatile Misuse. Goal: fix a DCL that omits volatile (the textbook broken version). Constraints: Java; keep the "check, lock, check" shape if you must keep DCL; one instance only.

// Before — DCL WITHOUT volatile: broken on the JMM. The poster child.
class Service {
    private static Service instance;          // <-- missing volatile

    static Service getInstance() {
        if (instance == null) {                // 1st check (no lock)
            synchronized (Service.class) {
                if (instance == null) {        // 2nd check (locked)
                    instance = new Service();  // <-- can be seen reordered
                }
            }
        }
        return instance;
    }
}

// After (minimal) — volatile makes the lock-free read safe.
class Service {
    private static volatile Service instance;   // volatile is mandatory

    static Service getInstance() {
        Service local = instance;                // read volatile once
        if (local == null) {
            synchronized (Service.class) {
                local = instance;
                if (local == null) {
                    local = new Service();
                    instance = local;            // volatile write publishes ctor effects
                }
            }
        }
        return local;
    }
}

Exercise 4 — Go "DCL" with a plain bool → Once¶

Anti-pattern: Double-Checked Locking (ported from Java, wrong in Go). Goal: kill a hand-ported DCL that uses an unsynchronized bool. Constraints: Go; one init only; preserve laziness.

// Before — DCL transliterated from Java; `done` is read without synchronization.
var (
    mu   sync.Mutex
    done bool
    conn *DB
)

func GetDB() *DB {
    if !done { // UNSYNCHRONIZED read — data race with the write below
        mu.Lock()
        if !done {
            conn = open()
            done = true
        }
        mu.Unlock()
    }
    return conn // also an unsynchronized read of conn
}

// After — sync.Once is the correct, lock-free-fast-path DCL.
var (
    once sync.Once
    conn *DB
)

func GetDB() *DB {
    once.Do(func() { conn = open() })
    return conn
}

Exercise 5 — volatile flag guarding a compound action¶

Anti-pattern: Volatile Misuse (treating volatile as a lock). Goal: fix a check-then-act that uses volatile for mutual exclusion. Constraints: Java; at most one transfer of pending into processed; preserve the result.

// Before — volatile gives visibility, NOT atomicity; this races.
class Inbox {
    private volatile boolean hasWork = false;
    private Work pending;
    private final List<Work> processed = new ArrayList<>();

    void submit(Work w) { pending = w; hasWork = true; }

    void drain() {
        if (hasWork) {            // check
            processed.add(pending); // act  <-- two threads can both pass the check
            hasWork = false;
        }
    }
}

// After — a lock makes check-and-act one indivisible step.
class Inbox {
    private final Object lock = new Object();
    private boolean hasWork = false;     // no longer needs volatile; lock guards it
    private Work pending;
    private final List<Work> processed = new ArrayList<>();

    void submit(Work w) {
        synchronized (lock) { pending = w; hasWork = true; }
    }

    void drain() {
        synchronized (lock) {
            if (hasWork) {
                processed.add(pending);
                hasWork = false;
            }
        }
    }
}

Exercise 6 — Non-atomic volatile counter → atomic¶

Anti-pattern: Volatile Misuse (volatile++ is not atomic). Goal: fix a lost-update counter. Constraints: Java; exact count under N concurrent incrementers; keep it lock-free if possible (this is an independent operation).

// Before — volatile makes ++ visible but NOT atomic: lost updates.
class Metrics {
    private volatile long hits = 0;

    void record() { hits++; }   // read, +1, write — interleaves and loses
    long hits()   { return hits; }
}

// After — AtomicLong: atomic increment, lock-free, correct.
class Metrics {
    private final AtomicLong hits = new AtomicLong();

    void record() { hits.incrementAndGet(); }
    long hits()   { return hits.get(); }
}

// Higher-contention variant: trades read cost for write throughput.
class MetricsHot {
    private final LongAdder hits = new LongAdder();
    void record() { hits.increment(); }
    long hits()   { return hits.sum(); }   // approximate during concurrent updates
}

Exercise 7 — Lazy cache map: lost writes → Once-per-key¶

Anti-pattern: Race-Prone Lazy Init (per key). Goal: lazily compute and cache an expensive value per key, building each value at most once. Constraints: Go; preserve Get(key) returning the memoized value; never build the same key twice concurrently.

// Before — concurrent Get on a cold key both miss, both build, one wins (work wasted + data race on the map).
type Cache struct {
    m map[string]*Val
}

func (c *Cache) Get(key string) *Val {
    if v, ok := c.m[key]; ok { // concurrent map read racing a write below
        return v
    }
    v := build(key) // expensive; multiple goroutines build the same key
    c.m[key] = v    // concurrent map write — fatal "concurrent map writes" panic
    return v
}

// After — per-key Once: build each key exactly once; no map race; concurrent keys parallel.
type entry struct {
    once sync.Once
    val  *Val
}

type Cache struct {
    mu sync.Mutex
    m  map[string]*entry
}

func NewCache() *Cache { return &Cache{m: make(map[string]*entry)} }

func (c *Cache) Get(key string) *Val {
    c.mu.Lock()
    e, ok := c.m[key]
    if !ok {
        e = &entry{}
        c.m[key] = e
    }
    c.mu.Unlock()

    e.once.Do(func() { e.val = build(key) }) // build off the map lock; once per key
    return e.val
}

Exercise 8 — atomic.Value config: torn compound update¶

Anti-pattern: Volatile Misuse / Wrong Memory Ordering (atomics chained without thinking). Goal: update two related config fields together so readers never see a half-updated config. Constraints: Go; readers are lock-free and frequent; writers are rare; a reader must see a consistent (host, port) pair.

// Before — two independent atomics; a reader can see new host with old port.
var host atomic.Pointer[string]
var port atomic.Int64

func Reload(h string, p int64) {
    host.Store(&h) // reader between these two lines sees (newHost, oldPort)
    port.Store(p)
}

func Endpoint() string {
    return *host.Load() + ":" + strconv.FormatInt(port.Load(), 10)
}

// After — one atomic pointer to an immutable snapshot: readers always consistent.
type Config struct {
    Host string
    Port int64
}

var current atomic.Pointer[Config]

func Reload(h string, p int64) {
    current.Store(&Config{Host: h, Port: p}) // one atomic publish of a complete value
}

func Endpoint() string {
    c := current.Load() // single atomic load → consistent (Host, Port)
    return c.Host + ":" + strconv.FormatInt(c.Port, 10)
}

func Bump() {
    for {
        old := current.Load()
        next := *old      // copy
        next.Port++
        if current.CompareAndSwap(old, &next) { return } // retry on lost race
    }
}

Exercise 9 — Python lazy init under the GIL¶

Anti-pattern: Race-Prone Lazy Init (survives the GIL). Goal: initialize a shared resource once across threads. Constraints: Python; one Connection for all callers; works on free-threaded 3.13+ too.

# Before — the GIL does NOT save you: the check and the assign can be interrupted.
_conn = None

def get_conn():
    global _conn
    if _conn is None:          # thread A passes the check...
        _conn = connect()      # ...switches out before assigning; thread B also builds one
    return _conn

# After — lock + recheck: built exactly once, GIL or no GIL.
import threading

_conn = None
_lock = threading.Lock()

def get_conn():
    global _conn
    if _conn is None:               # cheap fast path after init (no lock)
        with _lock:
            if _conn is None:       # recheck under the lock
                _conn = connect()
    return _conn

Exercise 10 — synchronized-everything → scoped locking + atomics¶

Anti-pattern: Over-synchronization (one giant lock) masking Volatile Misuse. Goal: keep correctness but stop one global lock from serializing two independent pieces of state. Constraints: Java; preserve all observable results; show the throughput win.

// Before — one lock serializes everything, including two unrelated fields.
class Stats {
    private long requests = 0;     // independent counter
    private final Map<String,Long> byUser = new HashMap<>(); // independent map

    synchronized void onRequest(String user) {           // global lock
        requests++;
        byUser.merge(user, 1L, Long::sum);
    }
    synchronized long requests() { return requests; }    // contends with map work
    synchronized long forUser(String u) { return byUser.getOrDefault(u, 0L); }
}

// After — each independent field gets its own correct, scoped primitive.
class Stats {
    private final AtomicLong requests = new AtomicLong();
    private final ConcurrentHashMap<String,Long> byUser = new ConcurrentHashMap<>();

    void onRequest(String user) {
        requests.incrementAndGet();          // independent, lock-free
        byUser.merge(user, 1L, Long::sum);   // atomic per-key, lock-striped internally
    }
    long requests()          { return requests.get(); }            // never blocks the map
    long forUser(String u)   { return byUser.getOrDefault(u, 0L); }
}

Exercise 11 — Keep the atomic — the counter-case¶

Anti-pattern: none — recognizing when a lock-free atomic is already the right answer (and resisting the urge to "add a mutex for safety"). Goal: review a single independent counter and decide. Constraints: Go; one monotonically increasing ID generator; correctness + max throughput.

// Before — already correct and optimal. A reviewer wants to "make it safe" with a Mutex.
var counter atomic.Uint64

func NextID() uint64 {
    return counter.Add(1) // atomic fetch-add: correct and lock-free
}

func TestNextID_Unique(t *testing.T) {
 const G, N = 64, 10000
 seen := make([]uint64, 0, G*N)
 var mu sync.Mutex // only to collect results, not under test
 var wg sync.WaitGroup
 for i := 0; i < G; i++ {
     wg.Add(1)
     go func() {
         defer wg.Done()
         local := make([]uint64, N)
         for j := range local { local[j] = NextID() }
         mu.Lock(); seen = append(seen, local...); mu.Unlock()
     }()
 }
 wg.Wait()
 set := make(map[uint64]struct{}, len(seen))
 for _, v := range seen { set[v] = struct{}{} }
 if len(set) != G*N { t.Fatalf("duplicate IDs: %d unique of %d", len(set), G*N) }
}

// Keep this — it is the optimization.
var counter atomic.Uint64
func NextID() uint64 { return counter.Add(1) }

// Reject this "safer" version — slower, no extra safety.
var (
    mu  sync.Mutex
    ctr uint64
)
func NextIDSlow() uint64 { mu.Lock(); ctr++; v := ctr; mu.Unlock(); return v }

Exercise 12 — The combo: broken DCL + volatile compound + lost init¶

Anti-pattern: all three at once. Goal: refactor a lazily-initialized, hand-DCL'd, flag-guarded counter store into a correct, fast implementation. Constraints: Go; preserve the API (Inc(key), Total()); build the store once; counts exact under load.

// Before — a trifecta of synchronization misuse.
var (
    mu      sync.Mutex
    ready   bool            // race-prone init flag (unsynchronized read below)
    store   map[string]*int64
)

func ensure() {
    if !ready { // unsynchronized read — broken Go "DCL"
        mu.Lock()
        if !ready {
            store = make(map[string]*int64)
            ready = true
        }
        mu.Unlock()
    }
}

func Inc(key string) {
    ensure()
    p := store[key]              // racy map read
    if p == nil {               // volatile-style check-then-act, but worse: no sync at all
        var z int64
        store[key] = &z          // racy map write — can panic
        p = &z
    }
    *p++                         // non-atomic increment — lost updates
}

func Total() (sum int64) {
    for _, p := range store { sum += *p } // racy iteration
    return
}

// After — Once for init, mutex for the map's compound ops, atomics for independent counts.
type Store struct {
    once sync.Once
    mu   sync.Mutex
    m    map[string]*atomic.Int64
}

var s Store

func (s *Store) init() {
    s.once.Do(func() { s.m = make(map[string]*atomic.Int64) })
}

func (s *Store) counter(key string) *atomic.Int64 {
    s.mu.Lock()
    defer s.mu.Unlock()
    c, ok := s.m[key]
    if !ok {
        c = new(atomic.Int64)
        s.m[key] = c
    }
    return c
}

func Inc(key string) {
    s.init()
    s.counter(key).Add(1) // map op under lock; increment is lock-free + atomic
}

func Total() int64 {
    s.init()
    s.mu.Lock()
    ptrs := make([]*atomic.Int64, 0, len(s.m)) // snapshot pointers under lock
    for _, c := range s.m {
        ptrs = append(ptrs, c)
    }
    s.mu.Unlock()
    var sum int64
    for _, c := range ptrs { // sum atomics off-lock
        sum += c.Load()
    }
    return sum
}

Refactoring discipline (concurrency) — the recap¶

Every exercise ran the same loop. Internalize it and the specific cures become mechanical:

reproduce with a tool  →  smallest correct primitive  →  verify race-free  →  (benchmark if perf was the goal)  →  commit

• Reproduce before you fix. A concurrency bug you can't trigger is a concurrency bug you can't prove fixed. Use go test -race (data races), jcstress (JMM outcomes), or a stress harness with the assertion as the oracle (logic races like Exercise 8, which -race won't catch). "Works on my machine" is the disease, not the cure.
• Reach for the weakest correct primitive.
• One-time init ⇒ sync.Once (Go) / holder idiom (Java) / lock+recheck (Python). Never hand-roll a DCL when the language gives you Once (Ex. 1, 2, 3, 4, 9).
• Single independent read-modify-write ⇒ one atomic (Ex. 6, 11). It is correct and fastest — adding a mutex would be cargo-cult locking.
• Compound action / check-then-act / cross-field invariant ⇒ a lock, or a single CAS on a single combined value (Ex. 5, 8, 12).
• volatile/atomic ≠ mutual exclusion. volatile (Java) and a lone atomic (Go) give visibility/ordering of one variable, never atomicity of a multi-step or multi-variable action. The instant two reads combine into a decision, you need a lock or a CAS loop (Ex. 5, 6, 8).
• Verify race-free, then verify fast. Re-run the detector/stress harness over many iterations — a sustained green is your "no data race" characterization test. Only then, where you claimed a speedup (Ex. 6, 10, 11), attach a benchmark. Never trade correctness for throughput; split a lock only when the state is genuinely independent (Ex. 10's caveat).
• Know when to stop. Sometimes the existing atomic is the optimization (Ex. 11). The disciplined refactor sometimes changes nothing but the test that proves it's already right.

Related Topics¶

• tasks.md — guided exercises building these cures from scratch.
• find-bug.md — the spotting counterpart: identify the race, don't fix it.
• junior.md · middle.md · senior.md · professional.md — recognize → detect → debug-in-prod → memory models.
• Coordination Anti-Patterns — why building under a held lock (Ex. 7) is the next mistake to avoid.
• Shared State Anti-Patterns — the root cause these synchronization fixes are all coordinating.
• Refactoring → Refactoring Techniques — the mechanical catalog for the structural moves used above.
• Clean Code → Immutability — the copy-on-write snapshot (Ex. 8) made systematic.