Part of duplicate code analysis: #2754

Summary

Across internal/logger/ and internal/server/, every mutex-protected method duplicates the same 2-line mu.Lock() / defer mu.Unlock() preamble before its actual logic. With 10+ types each having multiple such methods, this accounts for ~200 lines of structural boilerplate.

Duplication Details

Pattern: mu.Lock(); defer mu.Unlock() repeated in every guarded method

• Severity: Medium
• Occurrences: 20+ methods across 10+ files
• Locations:
  • internal/logger/file_logger.go (lines ~60–66, ~79–93, ~96–104)
  • internal/logger/jsonl_logger.go (lines ~71–76, ~84–101)
  • internal/logger/markdown_logger.go (lines ~73–89, ~108–168)
  • internal/logger/server_file_logger.go (similar pattern)
  • internal/server/session.go (lines ~68–101, ~104–114)
  • internal/guard/wasm.go (multiple mutex-guarded methods)

Code Sample — FileLogger.Close():

func (fl *FileLogger) Close() error {
    fl.mu.Lock()
    defer fl.mu.Unlock()
    return closeLogFile(fl.logFile, &fl.mu, "file")
}

Identical structure in JSONLLogger.Close():

func (jl *JSONLLogger) Close() error {
    jl.mu.Lock()
    defer jl.mu.Unlock()
    return closeLogFile(jl.logFile, &jl.mu, "JSONL")
}

session.go — double-check lock pattern also repeated:

us.sessionMu.RLock()
session := us.sessions[sessionID]
us.sessionMu.RUnlock()
if session == nil {
    us.sessionMu.Lock()
    if us.sessions[sessionID] == nil { /* init */ }
    us.sessionMu.Unlock()
}

This double-check pattern appears in multiple session-handling methods.

Impact Analysis

• Maintainability: If locking strategy changes (e.g., moving to sync.RWMutex or atomic), all call sites must be updated
• Bug Risk: Medium — copy-paste errors (e.g., locking wrong mutex) are possible
• Code Bloat: ~40 lines could be removed via withLock helpers; session double-check could use syncutil.Cache (already exists at internal/syncutil/cache.go)

Refactoring Recommendations

1. Add withLock helper methods to logger types:

   func (fl *FileLogger) withLock(fn func() error) error {
       fl.mu.Lock()
       defer fl.mu.Unlock()
       return fn()
   }
   // Usage:
   func (fl *FileLogger) Close() error {
       return fl.withLock(func() error {
           return closeLogFile(fl.logFile, &fl.mu, "file")
       })
   }

2. Use existing syncutil.Cache for the double-check locking pattern in session.go — the project already has internal/syncutil/cache.go for this purpose.

3. Estimated effort: 2–3 hours

Implementation Checklist

• Audit mutex-protected methods in internal/logger/ and internal/server/
• Add withLock / withRLock helpers where appropriate
• Migrate session double-check locking to syncutil.Cache
• Run make test-unit to confirm concurrency behavior is unchanged
• Run make agent-finished

Parent Issue

See parent analysis report: #2754
Related to #2754

Generated by Duplicate Code Detector · ◷

• expires on Apr 5, 2026, 6:00 AM UTC