Skip to content

CWE-362: Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition') - Go

Overview

Goroutines make concurrent access trivial to introduce and easy to miss: a shared variable, map, or struct field read and written from multiple goroutines without synchronization is a data race, and Go's own race detector (go test -race, go run -race) will catch many of them if it is actually run. The primary fix is sync.Mutex/sync.RWMutex around the full read-modify-write sequence, or sync/atomic - including the typed atomic.Int64/atomic.Bool wrapper types added in Go 1.19 - for simple counters and flags. When the shared state is a database row rather than an in-memory value, which is the common case for an HTTP handler shared across requests, use a transaction with SELECT ... FOR UPDATE or a single atomic UPDATE statement instead of a Go-level lock, since a sync.Mutex only protects goroutines inside one process.

Common Vulnerable Patterns

Unsynchronized Package-Level Counter

package stats

// VULNERABLE - requestCount is incremented from every request's goroutine
// with no synchronization
var requestCount int

func HandleRequest() {
    requestCount++ // read-modify-write: not atomic
}

// Attack: fire 1,000 concurrent requests, each calling HandleRequest() once.
// Result: requestCount often ends up less than 1,000 - increments are lost,
// and `go test -race` reports a DATA RACE on this line.

Why this is vulnerable: requestCount++ compiles to a separate load, add, and store. If two goroutines interleave between the load and the store, both compute the same new value from the same stale read and one increment is lost. The race detector flags it even on runs where the lost-update symptom does not appear, which is the point of running it: the absence of a wrong total is not evidence the race is absent.

Go's memory model is deliberately narrower than C's about what a race means, and the difference matters when triaging. Go does not permit a data race to invalidate the whole program: an implementation may report the race and stop, and otherwise a read of a single-word value must return some value actually written to that location, rather than a value out of thin air. So an int counter under a race loses increments and does not produce nonsense. That guarantee does not extend to multiword values - an interface, a slice, a string, a map header - where a torn read can combine one write's pointer with another's length or type, which is how a racy map or interface assignment turns into a crash rather than a wrong number.

Check-Then-Act on a Struct Field

package wallet

// VULNERABLE - check and update are two separate, unsynchronized steps
type Account struct {
    Balance int
}

func (a *Account) Withdraw(amount int) error {
    if a.Balance < amount {
        return errors.New("insufficient funds")
    }
    // RACE WINDOW: another goroutine can withdraw here before this line runs
    a.Balance -= amount
    return nil
}

// Attack: two goroutines call Withdraw(100) concurrently on an Account with
// Balance = 100. Both read Balance = 100 and both pass the check before
// either writes. Result: Balance ends at -100 instead of one call failing.

Why this is vulnerable: Nothing prevents two goroutines from being inside Withdraw at the same time, both having read the same starting balance before either one writes the decremented value back.

Separate SELECT Then UPDATE Without a Transaction

package inventory

// VULNERABLE - the read and the write are two independent database round-trips
func Reserve(db *sql.DB, sku string, quantity int) error {
    var stock int
    err := db.QueryRow("SELECT stock FROM inventory WHERE sku = $1", sku).Scan(&stock)
    if err != nil {
        return err
    }

    if stock < quantity {
        return errors.New("insufficient stock")
    }

    // RACE WINDOW: another request's SELECT/UPDATE pair can run here
    _, err = db.Exec("UPDATE inventory SET stock = stock - $1 WHERE sku = $2", quantity, sku)
    return err
}

// Attack: two concurrent Reserve(db, "sku-1", 10) calls when stock is 10.
// Both SELECTs see stock = 10, both UPDATEs subtract 10.
// Result: stock ends at -10 - both reservations succeeded against one unit of stock.

Why this is vulnerable: The SELECT and UPDATE run as two separate statements with no lock or transaction tying them together, so a concurrent request's SELECT can read the same pre-decrement value before this request's UPDATE commits.

Secure Patterns

sync.Mutex for a Struct's Critical Section

package wallet

import (
    "errors"
    "sync"
)

// SECURE - mutex protects the full read-modify-write sequence, and the
// balance is unexported so no caller can reach it around the mutex
type Account struct {
    mu      sync.Mutex
    balance int

    // nil in production. The test sets it to pin the interleaving; see Testing.
    betweenCheckAndWrite func()
}

func NewAccount(initialBalance int, betweenCheckAndWrite func()) *Account {
    return &Account{balance: initialBalance, betweenCheckAndWrite: betweenCheckAndWrite}
}

func (a *Account) Withdraw(amount int) error {
    a.mu.Lock()
    defer a.mu.Unlock()

    if a.balance < amount {
        return errors.New("insufficient funds")
    }
    if a.betweenCheckAndWrite != nil {
        a.betweenCheckAndWrite()
    }
    a.balance -= amount
    return nil
}

// Balance takes the same mutex: an unguarded read has no happens-before edge
// to the writes above and can observe a stale value indefinitely.
func (a *Account) Balance() int {
    a.mu.Lock()
    defer a.mu.Unlock()
    return a.balance
}

Why this works: mu.Lock() blocks every other goroutine from entering the method body until Unlock() runs, so the balance check and the decrement always execute as one atomic unit relative to other callers. defer a.mu.Unlock() releases the lock on every exit path, so a panic or an early return added by a later change cannot leave the mutex held.

Two details that are easy to drop and change the outcome. The field is unexported: an exported Balance int sitting next to the mutex is readable and writable by every caller in the package's importers without touching the lock, so the type advertises synchronization it cannot enforce - and the accessor below is what the test needs anyway. And the accessor locks, because -race correctly reports an unsynchronized read of a field that other goroutines write, even when the read is "only" in a test.

The betweenCheckAndWrite hook is not part of the fix; it is the seam the concurrency test needs to reproduce the race deterministically, and it is here rather than in the test file because the window it has to open is inside the method. It costs one nil check and nothing else in production.

sync/atomic for a Simple Counter

package stats

import "sync/atomic"

// SECURE - atomic.Int64 avoids the need for a lock on a single counter
var requestCount atomic.Int64

func HandleRequest() {
    requestCount.Add(1)
}

func CurrentCount() int64 {
    return requestCount.Load()
}

Why this works: atomic.Int64.Add performs the read, the add and the write as one atomic operation - there is no window in which two goroutines can both read the same value before either writes it back. The typed atomic.Int64 (Go 1.19+) also prevents the common mistake of accidentally reading or writing the underlying int64 without going through the atomic package, which the older atomic.AddInt64(&count, 1) pointer-based API does not guard against.

sync.Map or a Mutex-Guarded Map for Shared Collections

package cache

import "sync"

// SECURE - sync.Map is safe for concurrent access, but a compound
// check-then-set still needs its own atomicity
type SessionCache struct {
    sessions sync.Map // map[string]*Session
}

func (c *SessionCache) GetOrCreate(id string, create func() *Session) *Session {
    // LoadOrStore is itself atomic: only one goroutine's create() result wins
    // for a given key, even if multiple goroutines race to create the same id
    actual, _ := c.sessions.LoadOrStore(id, create())
    return actual.(*Session)
}

Why this works: A plain map is never safe for concurrent reads and writes in Go - even one writer racing with one reader corrupts the map's internal state and can crash the program (fatal error: concurrent map read and map write), which sync.Mutex-protected access or sync.Map both prevent. LoadOrStore additionally makes the "check if a session exists, otherwise create one" sequence atomic in a single call, closing the race that a separate Load followed by Store would leave open.

create() above runs on every call, including the ones that lose. Go evaluates arguments before the call, so LoadOrStore(id, create()) constructs a session whether or not it is stored, and every loser's session is discarded. For a cheap value that is only wasted allocation. For a session it is usually not: if create() mints an identifier, writes a row, or takes a slot from a quota, the discarded ones are real and the side effects survive the discard - the same shape as a side-effecting AddOrUpdate factory. Where creation costs anything or does anything, do a Load first for the common hit path and fall back to a per-key sync.Mutex (or golang.org/x/sync/singleflight) around the check-and-create, so exactly one goroutine constructs and the rest wait for its result.

Database Transaction with SELECT ... FOR UPDATE

package inventory

import (
    "context"
    "database/sql"
    "errors"
)

// SECURE - the row lock is held from the SELECT through the UPDATE, inside
// one transaction, so no other transaction can read a stale stock value
func Reserve(ctx context.Context, db *sql.DB, sku string, quantity int) error {
    tx, err := db.BeginTx(ctx, nil)
    if err != nil {
        return err
    }
    defer tx.Rollback() // no-op if Commit succeeds

    var stock int
    err = tx.QueryRowContext(ctx,
        "SELECT stock FROM inventory WHERE sku = $1 FOR UPDATE", sku,
    ).Scan(&stock)
    if err != nil {
        return err
    }

    if stock < quantity {
        return errors.New("insufficient stock")
    }

    _, err = tx.ExecContext(ctx,
        "UPDATE inventory SET stock = stock - $1 WHERE sku = $2", quantity, sku)
    if err != nil {
        return err
    }

    return tx.Commit()
}

Why this works: FOR UPDATE takes a row-level lock at the SELECT, and the transaction holds that lock until Commit() or Rollback(). Any other transaction's SELECT ... FOR UPDATE on the same row blocks until this transaction finishes, so the stock value this code checked cannot change out from under it before the UPDATE runs. This works correctly across every process and server instance connected to the database, not just within one Go process.

Atomic Conditional UPDATE (No Row Lock Needed)

package wallet

import (
    "context"
    "database/sql"
    "errors"
)

// SECURE - the precondition and the write happen in one atomic statement
func Withdraw(ctx context.Context, db *sql.DB, accountID string, amount int) error {
    result, err := db.ExecContext(ctx,
        `UPDATE accounts SET balance = balance - $1
         WHERE id = $2 AND balance >= $1`,
        amount, accountID)
    if err != nil {
        return err
    }

    rows, err := result.RowsAffected()
    if err != nil {
        return err
    }
    if rows == 0 {
        return errors.New("insufficient funds or account not found")
    }
    return nil
}

Why this works: There is no separate read at all - the database evaluates balance >= $1 and performs the subtraction as part of the same atomic statement. RowsAffected() == 0 reliably distinguishes "the precondition failed" from "the write succeeded," without ever needing an explicit lock, transaction, or round-trip for the check.

Framework-Specific Guidance

net/http

// SECURE - shared state behind an http.Handler is protected the same way as
// any other goroutine-shared state, since net/http serves each request on
// its own goroutine
type CounterHandler struct {
    mu    sync.Mutex
    count int
}

func (h *CounterHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    h.mu.Lock()
    h.count++
    current := h.count
    h.mu.Unlock()

    fmt.Fprintf(w, "request #%d\n", current)
}

Why this works: net/http dispatches each incoming request on its own goroutine by design, so any field on a handler struct shared across requests is exactly the kind of state this CWE describes. Guarding it with a mutex (or replacing the counter with atomic.Int64, as above) makes the handler safe regardless of how many requests arrive concurrently.

database/sql

// SECURE - always set a bounded connection pool and a context deadline so
// a held row lock cannot stall the whole service under contention
db.SetMaxOpenConns(25)
db.SetConnMaxLifetime(5 * time.Minute)

ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()

if err := Reserve(ctx, db, sku, quantity); err != nil {
    // handle insufficient stock or timeout
}

Why this works: SELECT ... FOR UPDATE blocks contending transactions until the lock is released; without a bounded pool and a context deadline, a backlog of blocked transactions can exhaust connections and cascade into an outage. Passing a context with a timeout ensures a stuck transaction is cancelled rather than held indefinitely.

Considerations

A lock is only as wide as the thing holding it. An in-process lock serialises the threads inside one instance and does nothing about a second instance, so a fix that works on a developer machine can fail the moment the service is scaled out or restarted behind a load balancer. Decide first where the shared state actually lives. If it is a database row, the serialisation has to happen in the database - a row lock, a conditional update, or a version column. If it is genuinely in-process and stays that way, an in-process lock is correct and cheaper.

Not every race is worth fixing. Two requests overwriting a display preference, or a page-view counter losing an increment, is a race with no security consequence and often no user-visible one. The ones that matter change a decision: a balance check, a quota, a one-time token being redeemed, a permission being evaluated. Fixing a benign race costs throughput and adds a failure mode, so say which category the finding is in before reaching for a lock.

go test -race only reports races the test actually exercises. It is a detector, not a prover: a clean run means the interleavings you drove were clean. Write the test so it genuinely contends - many goroutines on the same key, not one after another - and treat a clean result as evidence rather than proof.

Optimistic and pessimistic locking fail in opposite directions. A pessimistic lock makes every caller wait, so it is predictable but caps throughput and can deadlock if two paths take locks in different orders. Optimistic concurrency lets callers proceed and rejects the loser, which is faster when conflicts are rare and degenerates into wasted work and retries when they are common. Pick by how often the same row is genuinely contended, not by which is easier to write.

Retries need a bound and a backoff. A conflict-and-retry loop with neither turns a contended row into a livelock under load - every caller retrying immediately, none making progress. Cap the attempts, back off between them, and decide what the caller sees when the cap is reached. "Try again" is a legitimate answer; silently returning stale data is not.

Testing

A re-scan cannot confirm this fix. A linter sees a sync.Mutex where there was none and reports the finding closed; it cannot tell whether the critical section covers the whole decision, or whether the locked version still serves a legitimate request. go test -race is much better but answers a different question - it reports unsynchronized memory access, not a broken invariant - so a check-then-act that is correctly locked over a database round trip, or one where the two goroutines happen not to overlap on this run, produces a clean race report and a negative balance.

Launching goroutines together does not reproduce the race. The window between the check and the write is a few nanoseconds, and the goroutines usually run to completion one after another. Measured on Go 1.25 with GOMAXPROCS=16, five goroutines against the unsynchronized Account returned the "correct" answer of 3 on 498 runs out of 500.

Pin the interleaving. Give the type under test a hook between the check and the write, and have the hook hold every caller that got past the check until all of them have arrived - or until a timeout elapses, which is what keeps the correctly locked implementation from deadlocking:

package wallet_test

import (
    "sync"
    "testing"
    "time"

    "example.com/app/wallet"
)

// gate returns a hook that holds each caller which passed the balance check
// until `callers` of them have arrived, or until `hold` elapses.
//
// The timeout is load-bearing. Under a working mutex only one caller is ever
// inside Withdraw, so the arrival count never reaches `callers` - without the
// timeout the test would hang against the code it is supposed to pass.
func gate(callers int, hold time.Duration) func() {
    arrived := make(chan struct{}, callers)
    release := make(chan struct{})
    go func() {
        for i := 0; i < callers; i++ {
            <-arrived
        }
        close(release)
    }()
    return func() {
        arrived <- struct{}{}
        select {
        case <-release:
        case <-time.After(hold):
        }
    }
}

func TestConcurrentWithdrawals_NeverOverdraftTheAccount(t *testing.T) {
    const callers = 5
    acct := wallet.NewAccount(100, gate(callers, 100*time.Millisecond))

    var wg sync.WaitGroup
    results := make([]error, callers)
    for i := 0; i < callers; i++ {
        wg.Add(1)
        go func(i int) {
            defer wg.Done()
            results[i] = acct.Withdraw(30)
        }(i)
    }
    wg.Wait()

    succeeded := 0
    for _, err := range results {
        if err == nil {
            succeeded++
        }
    }

    // Exactly 3 of the 5 concurrent withdrawals of 30 succeed against 100,
    // and the balance lands on the arithmetic those 3 imply.
    if succeeded != 3 {
        t.Errorf("expected exactly 3 successful withdrawals, got %d", succeeded)
    }
    if got := acct.Balance(); got != 10 {
        t.Errorf("expected a final balance of 10, got %d", got)
    }
}

This needs two things from the production type: a constructor that accepts an optional hook (nil in production), and a Balance() accessor that takes the same mutex. Reading acct.Balance as an exported field from the test is itself a data race, and go test -race will fail the run for it - reporting the test rather than the code, which is how a real finding gets dismissed as noise.

Measured on Go 1.25: 100 gated runs against the unsynchronized Withdraw produced 5 successes and a balance of -50 on every one, and 20 runs against the mutex version produced 3 and 10 on every one. The test costs one hold per successful caller when the code is correct - 300 ms here - which is the price of a concurrency test that fails when it should.

A first draft of this test used a sync.WaitGroup for the arrivals and a bare <-gate with no timeout. It reproduced the race perfectly and hung forever against the fixed code, because a working mutex means the fifth arrival never happens. Worth generalising: any test that forces an interleaving is asserting that several callers are inside the section at once, which is exactly what the fix makes false. Give every such rendezvous a timeout, and run the test against the fixed code before believing it.

Assert these, with the result each should produce:

  • Accept, single call. Withdraw(30) against 100 returns nil and Balance() is exactly 70. A mutex that refuses every caller passes the concurrency assertion above and fails this one.
  • Accept, at the boundary. Withdraw(100) against 100 returns nil and leaves 0.
  • Reject, past the boundary. Withdraw(1) against 0 returns a non-nil error and leaves Balance() at 0 - unchanged, not merely non-negative.
  • Concurrent. The gated test above: 3 successes and Balance() == 10. Assert the balance as well as the count; a lost update can produce the right number of successes and the wrong total.
  • Copying the value does not silently unlock it. A test that passes Account by value rather than by pointer gets its own mutex and its own balance, so every caller succeeds. go vet reports the copy - run it, and do not skip its sync warnings.
  • Run every package with go test -race ./... in CI, not just locally. The detector only reports races the run actually performs, so it needs a test that genuinely contends - which is what the hook above guarantees.

For state in a database, none of the above reaches the real weakness - a sync.Mutex is invisible to a second process. Drive the endpoint from several processes and assert on the persisted row.

Common Pitfalls

  • Guarding one field with a mutex while a sibling field on the same struct is mutated without it: Typically the mutex arrives with a new field and the older, related one is left as it was: the race on the older field is unchanged, and an invariant that spans both fields can still break even when each field has its own lock.
  • Copying a struct that contains a sync.Mutex or sync.Map: Passing an Account by value instead of by pointer copies the mutex too, so callers can end up locking two different mutex instances and get no mutual exclusion at all; go vet flags this, so do not ignore its warnings on types containing sync primitives.
  • Using atomic operations on some accesses to a variable but plain reads/writes elsewhere: Mixing atomic.Int64.Add() in one function with a direct count++ in another still races - every access to the shared variable has to go through the same synchronization mechanism, consistently.
  • Treating a sync.RWMutex read lock as sufficient for a check-then-act sequence: RLock() allows multiple concurrent readers, so if the "check" is done under RLock() and the "act" (the write) is done separately under Lock(), another reader or writer can interleave between them; the entire check-then-act sequence needs to be under a single Lock() (or the check redone after acquiring the write lock).

Dependencies and Installation

No additional module is required for sync.Mutex, sync.RWMutex, sync.Map, or sync/atomic - all are part of the Go standard library. database/sql transactions and FOR UPDATE work with any SQL driver already in use (pgx, lib/pq, go-sql-driver/mysql); no extra package is needed beyond the driver already required for database access.

Additional Resources