[duplicate-code] Duplicate Code Pattern: Double-Check Locking in Concurrent Cache Lookups

[github-actions]

Part of duplicate code analysis: #aw_parent001main

Summary

The read-lock → release → write-lock → double-check pattern for concurrent cache/map access is copy-pasted verbatim into at least three separate files with only cosmetic differences (variable names, log messages). Any bug in the pattern (e.g. a missed double-check) must be fixed in all three places independently.

Duplication Details

Pattern: RWMutex double-check cache lookup

• Severity: High
• Occurrences: 3 (+ potential future instances as new caches are added)
• Locations:
  • internal/launcher/launcher.go — GetOrLaunch / GetOrLaunchForSession methods (~lines 56–76, 109–130)
  • internal/launcher/connection_pool.go — getOrCreateLogger method (~lines 171–187)
  • internal/server/routed.go — getOrCreate method (~lines 46–63)

launcher.go:

l.mu.RLock()
if conn, ok := l.connections[serverID]; ok {
    l.mu.RUnlock()
    return conn, nil
}
l.mu.RUnlock()

l.mu.Lock()
defer l.mu.Unlock()

// Double-check after acquiring write lock
if conn, ok := l.connections[serverID]; ok {
    return conn, nil
}
// … create and store conn

connection_pool.go:

sfl.mu.RLock()
if logger, exists := sfl.loggers[serverID]; exists {
    sfl.mu.RUnlock()
    return logger, nil
}
sfl.mu.RUnlock()

sfl.mu.Lock()
defer sfl.mu.Unlock()

if logger, exists := sfl.loggers[serverID]; exists {
    return logger, nil
}
// … create and store logger

routed.go:

c.mu.RLock()
if server, exists := c.servers[key]; exists {
    c.mu.RUnlock()
    return server
}
c.mu.RUnlock()

c.mu.Lock()
defer c.mu.Unlock()

if server, exists := c.servers[key]; exists {
    return server
}
// … create and store server

Impact Analysis

• Maintainability: Any fix (e.g., add instrumentation, change locking strategy) must be applied to all three copies
• Bug Risk: A developer adding a fourth cache is likely to copy one of these, perpetuating the pattern further
• Code Bloat: ~15 lines × 3 occurrences = ~45 lines of near-identical code

Refactoring Recommendations

1. Extract a generic getOrCreate helper (Go 1.18+ generics)

   // internal/syncutil/cache.go
   func GetOrCreate[K comparable, V any](
       mu *sync.RWMutex,
       cache map[K]V,
       key K,
       create func() (V, error),
   ) (V, error) {
       mu.RLock()
       if v, ok := cache[key]; ok {
           mu.RUnlock()
           return v, nil
       }
       mu.RUnlock()
   
       mu.Lock()
       defer mu.Unlock()
       if v, ok := cache[key]; ok {
           return v, nil
       }
       v, err := create()
       if err == nil {
           cache[key] = v
       }
       return v, err
   }

   Estimated effort: ~2 hours
   Benefits: single correctness-verified implementation, trivially unit-testable, enforces consistent concurrency semantics

2. Alternative: use sync.Map where a simpler LoadOrStore idiom suffices and avoid manual locking entirely.

Implementation Checklist

• Review all three duplication sites
• Create internal/syncutil (or similar) package with generic helper
• Migrate launcher.go, connection_pool.go, and routed.go
• Add unit tests for the helper under race detector (go test -race)
• Verify no functionality broken

Parent Issue

See parent analysis report: #aw_parent001main

Generated by Duplicate Code Detector · ◷

• expires on Apr 1, 2026, 3:28 AM UTC