[duplicate-code] Duplicate Code Pattern: Mutex Lock/Defer Unlock Boilerplate

[github-actions]

🔍 Duplicate Code Pattern: Mutex Lock/Defer Unlock Boilerplate

Part of duplicate code analysis: #264

Summary

The codebase contains 80+ instances of the same mutex lock/defer unlock pattern across multiple packages. While this is idiomatic Go, the repetition creates maintenance burden and increases the risk of lock-related bugs (forgetting defer, wrong lock type, etc.).

Duplication Details

Pattern: Lock/Defer Unlock Boilerplate

• Severity: Medium
• Occurrences: 80+ instances
• Affected Packages:
  • internal/logger/ - 25 instances
  • internal/difc/ - 28 instances
  • internal/guard/ - 9 instances
  • internal/server/ - 5 instances
  • internal/launcher/ - 3 instances

Code Sample (from multiple files):

// Pattern 1: Write lock
func SomeFunction() {
	globalLoggerMu.Lock()
	defer globalLoggerMu.Unlock()
	// ... critical section ...
}

// Pattern 2: Read lock
func AnotherFunction() {
	globalLoggerMu.RLock()
	defer globalLoggerMu.RUnlock()
	// ... read-only access ...
}

Examples across packages:

From internal/logger/file_logger.go:

func LogInfo(category, format string, args ...interface{}) {
	globalLoggerMu.RLock()
	defer globalLoggerMu.RUnlock()

	if globalFileLogger != nil {
		globalFileLogger.Log(LogLevelInfo, category, format, args...)
	}
}

From internal/difc/capabilities.go:

func (c *Capabilities) Set(key string, value bool) {
	c.mu.Unlock()
	defer c.mu.Unlock()
	c.caps[key] = value
}

From internal/guard/registry.go:

func (r *Registry) Register(id string, guard Guard) {
	r.mu.Unlock()
	defer r.mu.Unlock()
	r.guards[id] = guard
}

Impact Analysis

• Maintainability: Medium impact - Pattern is verbose but well-understood in Go
• Bug Risk: Low-Medium - Easy to make mistakes:
  • Using Lock() instead of RLock() for read-only operations
  • Forgetting defer (caught by go vet, but still possible)
  • Holding locks longer than necessary
• Code Bloat: ~160 lines of repetitive lock/unlock code
• Performance: Potential over-locking where granular locks aren't needed

Refactoring Recommendations

1. Helper Functions for Common Lock Patterns

• Create utility functions for common lock/execute/unlock patterns
• Estimated effort: 2-3 hours
• Benefits:
  • Reduced boilerplate
  • Guaranteed proper lock handling
  • More concise code

Suggested implementation:

// WithReadLock executes fn while holding a read lock
func WithReadLock(mu *sync.RWMutex, fn func()) {
	mu.RLock()
	defer mu.RUnlock()
	fn()
}

// WithWriteLock executes fn while holding a write lock
func WithWriteLock(mu *sync.RWMutex, fn func()) {
	mu.Lock()
	defer mu.Unlock()
	fn()
}

// WithReadLockReturn executes fn and returns its result while holding a read lock
func WithReadLockReturn[T any](mu *sync.RWMutex, fn func() T) T {
	mu.RLock()
	defer mu.RUnlock()
	return fn()
}

// Usage example:
func LogInfo(category, format string, args ...interface{}) {
	WithReadLock(&globalLoggerMu, func() {
		if globalFileLogger != nil {
			globalFileLogger.Log(LogLevelInfo, category, format, args...)
		}
	})
}

2. Consider sync.Map for Simple Cases

• For registries and caches, use sync.Map to eliminate manual locking
• Estimated effort: 4-6 hours
• Benefits:
  • No manual lock management
  • Better performance for read-heavy workloads
  • Thread-safe by design

Example for Registry pattern:

// Before: Manual locking
type Registry struct {
	guards map[string]Guard
	mu     sync.RWMutex
}

func (r *Registry) Register(id string, guard Guard) {
	r.mu.Lock()
	defer r.mu.Unlock()
	r.guards[id] = guard
}

// After: Using sync.Map
type Registry struct {
	guards sync.Map // map[string]Guard
}

func (r *Registry) Register(id string, guard Guard) {
	r.guards.Store(id, guard)
}

3. Extract Locked Operations to Methods

• Encapsulate critical sections in well-named methods
• Estimated effort: 3-4 hours
• Benefits:
  • Clearer intent
  • Reduced lock scope
  • Easier to test

Example:

// Before: Lock in calling code
func ProcessRequest() {
	globalLoggerMu.RLock()
	if globalFileLogger != nil {
		// ... use globalFileLogger ...
	}
	globalLoggerMu.RUnlock()
}

// After: Lock encapsulated in method
func getFileLogger() *FileLogger {
	globalLoggerMu.RLock()
	defer globalLoggerMu.RUnlock()
	return globalFileLogger
}

func ProcessRequest() {
	logger := getFileLogger()
	if logger != nil {
		// ... use logger ...
	}
}

Priority Assessment

Priority: Medium-Low

While this pattern creates visual clutter and minor maintenance overhead, it is:

• Idiomatic Go code
• Generally safe when used correctly
• Not causing known bugs

Recommendation:

• Address this during major refactoring efforts
• Focus on high-risk areas first (e.g., logger initialization from sub-issue [duplicate-code] Duplicate Code Pattern: Logger Initialization and Cleanup #265)
• Consider helpers for new code, but avoid mass refactoring existing working code

Implementation Checklist

• Evaluate if refactoring effort is justified
• Create internal/syncutil/ package with lock helpers if proceeding
• Identify highest-value targets (most complex critical sections)
• Apply helpers to new code first
• Gradually refactor existing code during other changes
• Consider sync.Map for registry patterns
• Update coding guidelines to recommend helpers
• Add linter rules to detect common locking mistakes

Parent Issue

See parent analysis report: #264
Related to #264

AI generated by Duplicate Code Detector

[github-actions]

This issue was automatically closed because it expired on 2026-01-15T17:13:11.563Z.