go-concurrency-review
Installation
SKILL.md
Go Concurrency Review
Concurrency in Go is powerful and deceptively easy to get wrong. These patterns prevent goroutine leaks, data races, and deadlocks.
1. Goroutine Lifecycle Management
EVERY goroutine MUST have a clear termination path. No fire-and-forget.
Use errgroup for coordinated goroutines:
g, ctx := errgroup.WithContext(ctx)
g.Go(func() error {
return fetchUsers(ctx)
})
g.Go(func() error {
return fetchOrders(ctx)
})
if err := g.Wait(); err != nil {
return fmt.Errorf("fetch data: %w", err)
}
Long-running goroutines must respect context:
func (w *Worker) Run(ctx context.Context) error {
for {
select {
case <-ctx.Done():
return ctx.Err()
case job := <-w.jobs:
if err := w.process(job); err != nil {
w.logger.Error("process job", slog.Any("error", err))
}
}
}
}
Start goroutines in the owner, not the callee:
// ✅ Good — caller controls lifecycle
go worker.Run(ctx)
// ❌ Bad — function secretly starts goroutine
func NewWorker() *Worker {
w := &Worker{}
go w.run() // hidden goroutine — caller has no control
return w
}
2. Channel Patterns
Channel size is one or none:
// Unbuffered — synchronization point
ch := make(chan Result)
// Buffered with size 1 — single-item handoff
ch := make(chan Result, 1)
// Larger buffers need explicit justification with documented reasoning
ch := make(chan Result, 100) // requires comment explaining why
Signal channels use empty struct:
done := make(chan struct{})
close(done) // broadcast signal to all receivers
Producer/consumer with clean shutdown:
func produce(ctx context.Context) <-chan Item {
ch := make(chan Item)
go func() {
defer close(ch)
for {
item, err := fetchNext(ctx)
if err != nil {
return
}
select {
case ch <- item:
case <-ctx.Done():
return
}
}
}()
return ch
}
3. Mutex Patterns
Zero-value mutexes are valid:
// ✅ Good — zero value works
type Cache struct {
mu sync.RWMutex
items map[string]Item
}
// ❌ Bad — unnecessary pointer
type Cache struct {
mu *sync.RWMutex // never do this
}
Mutex placement in struct:
type SafeMap struct {
mu sync.RWMutex // mutex guards the fields below
items map[string]string
count int
}
The mutex should appear directly above the field(s) it protects, with a comment indicating the relationship.
Lock scope should be minimal:
// ✅ Good — minimal lock scope
func (c *Cache) Get(key string) (Item, bool) {
c.mu.RLock()
item, ok := c.items[key]
c.mu.RUnlock()
return item, ok
}
// ✅ Also good — defer for methods that return early
func (c *Cache) GetOrCreate(key string) Item {
c.mu.Lock()
defer c.mu.Unlock()
if item, ok := c.items[key]; ok {
return item
}
item := newItem(key)
c.items[key] = item
return item
}
Never copy mutexes:
// ❌ BLOCKER — copying a mutex copies its lock state
cache2 := *cache1 // this copies the mutex!
4. Atomic Operations
Use sync/atomic or go.uber.org/atomic for simple counters and flags:
// ✅ Good — type-safe atomics
import "go.uber.org/atomic"
type Server struct {
running atomic.Bool
reqCount atomic.Int64
}
func (s *Server) HandleRequest() {
s.reqCount.Inc()
// ...
}
5. Context Propagation
Rules:
- Context is ALWAYS the first parameter.
- Never store context in a struct field.
- Derive child contexts for sub-operations:
func (s *Service) Process(ctx context.Context, req Request) error {
// Derive context with timeout for external call
fetchCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel() // ALWAYS defer cancel
data, err := s.client.Fetch(fetchCtx, req.ID)
if err != nil {
return fmt.Errorf("fetch %s: %w", req.ID, err)
}
// ...
}
NEVER ignore context cancellation in select:
// ✅ Good
select {
case result := <-ch:
return result, nil
case <-ctx.Done():
return nil, ctx.Err()
}
// ❌ Bad — blocks forever if context cancelled
result := <-ch
6. Avoid Mutable Globals
// ❌ Bad — mutable global, not safe for concurrent access
var db *sql.DB
// ✅ Good — pass as dependency
type Server struct {
db *sql.DB
}
7. sync.Once for Lazy Initialization
type Client struct {
initOnce sync.Once
conn *grpc.ClientConn
}
func (c *Client) getConn() *grpc.ClientConn {
c.initOnce.Do(func() {
c.conn = dial()
})
return c.conn
}
Race Detection
ALWAYS run tests with race detector during CI:
go test -race ./...
This is non-negotiable. A test suite that passes without -race proves nothing
about concurrent correctness.
Red Flags Checklist
- 🔴 Goroutine started without shutdown path
- 🔴 Channel never closed (potential goroutine leak)
- 🔴 Mutex copied by value
- 🔴 Context stored in struct field
- 🔴
context.Background()used where parent context was available - 🔴
selectwithoutctx.Done()case in blocking operation - 🔴 Shared map/slice accessed without synchronization
- 🟡 Buffered channel with arbitrary large size
- 🟡
time.Sleepused for synchronization instead of proper signaling - 🟡 Goroutine starting inside
init()or constructor without lifecycle control
Related skills