Concurrency Patterns#

Concurrency Patterns in the opnDossier project represent a collection of thread-safety and goroutine management techniques used to ensure safe concurrent access to shared data structures in Go. The opnDossier project, a configuration analyzer for OPNsense firewalls, implements several critical patterns including RWMutex-based protection with non-reentrant workarounds, sync.Once singleton initialization, context-aware cancellation, and robust goroutine lifecycle management. These patterns address fundamental challenges in Go's concurrency model, particularly the non-reentrant nature of sync.RWMutex, lazy initialization of shared resources with memory ordering guarantees, and the need for safe shutdown coordination between goroutines and their callers.

The patterns documented here emerge from practical implementation needs in internal/processor/report.go (split into report.go, report_statistics.go, and report_markdown.go as of the file reorganization), internal/progress/spinner.go, and internal/audit/plugin.go, where concurrent report generation, UI updates, and plugin registry access require careful coordination. The project follows a philosophy of using concurrency judiciously—sequential processing remains the default, with concurrent patterns applied only where read-heavy workloads or UI responsiveness demands necessitate them.

RWMutex Protection#

Basic Pattern#

The Report struct uses sync.RWMutex to protect concurrent access to the Findings field:

type Report struct {
    mu sync.RWMutex `json:"-" yaml:"-"` // protects Findings for concurrent access
    // ... other fields ...
    Findings Findings `json:"findings"`
}

Key Principles:

• ALL read methods need RLock(), not just write methods
• RWMutex allows multiple concurrent readers or one exclusive writer
• Always defer unlock operations: defer r.mu.RUnlock()
• Exclude mutex from serialization with struct tags

Implementation Examples#

Multiple read methods throughout the codebase demonstrate proper RLock usage:

• TotalFindings() acquires RLock()
• ToJSON(), ToYAML() use RLock(); ToMarkdown() uses RLock()
• HasCriticalFindings() uses RLock() for read-only check

Non-Reentrant Lock Workaround#

The Problem#

Go's sync.RWMutex is NOT reentrant—a goroutine holding a lock cannot acquire the same lock again without deadlocking. This creates challenges when methods need to call other methods that also require lock protection.

The Solution: Unsafe Helper Methods#

The opnDossier codebase solves this through internal "Unsafe" helper methods that assume the lock is already held by the caller.

Pattern Structure:

• Public methods acquire the lock and call unsafe helpers
• Unsafe helpers assume the lock is already held by the caller
• Document unsafe methods with comments: "Caller must hold mu"

Real Implementation#

totalFindingsUnsafe() helper method:

// totalFindingsUnsafe returns total findings without locking. Caller must hold mu.
func (r *Report) totalFindingsUnsafe() int {
    return len(r.Findings.Critical) + len(r.Findings.High) +
        len(r.Findings.Medium) + len(r.Findings.Low) + len(r.Findings.Info)
}

Public method that acquires lock:

func (r *Report) TotalFindings() int {
    r.mu.RLock()
    defer r.mu.RUnlock()
    return r.totalFindingsUnsafe()
}

Another example: addFindingsUnsafe() is called from ToMarkdown() where RLock is already held.

Safe Data Return Patterns#

Value Copies vs Pointers#

Getter methods in thread-safe structs should return value copies, not pointers to internal state, to prevent callers from holding references to data that might be mutated concurrently.

Pattern in opnDossier:

• TotalFindings() returns int (value)
• HasCriticalFindings() returns bool (value)
• ToJSON(), ToYAML() return strings (values); ToMarkdown() returns string (value)

Guidelines:

• Return value copies for small types (primitives, strings, small structs)
• For slices: create new slice and copy elements if caller might mutate
• Avoid returning pointers to internal mutex-protected data
• When serializing (JSON/YAML), the serialized output is inherently a safe copy

Struct Shallow Copy and Slice Safety#

When copying structs that contain slices, be aware that normalized := *cfg creates a shallow copy—the struct is copied but slices share backing arrays.

Safe Patterns:

// Shallow copy - struct copied but slices share backing array
normalized := *cfg

// Deep copy slices you intend to mutate
safeRules := make([]Rule, len(cfg.Rules))
copy(safeRules, cfg.Rules)

In opnDossier:
The Findings struct contains slices, but the code avoids exposing shallow copies. Instead, it either:

• Serializes findings to new formats (JSON/YAML/Markdown)
• Returns aggregate values (counts, booleans)
• Initializes with fresh slices in NewReport()

Goroutine Lifecycle Management#

The Stopped Channel Pattern#

The stopped channel pattern documented in AGENTS.md §5.23 ensures write safety when both a goroutine and its stop method write to the same io.Writer.

The Problem:
When a goroutine writes to an output stream and a stop method also writes after signaling shutdown, concurrent writes can cause corruption or races.

The Solution:
Use dual channels for bidirectional coordination:

• done channel: stop method signals goroutine to terminate
• stopped channel: goroutine signals stop method that it has fully exited

Pattern from AGENTS.md:

func (s *Spinner) spin() {
    defer close(s.stopped) // signal goroutine exit
    // ... write loop ...
}

func (s *Spinner) stop() {
    close(s.done) // signal shutdown
    <-s.stopped // wait for goroutine to finish writing
}

Implementation in internal/progress/#

The SpinnerProgress struct implements this pattern:

type SpinnerProgress struct {
    running bool
    done chan struct{}
    stopped chan struct{} // closed when spin goroutine exits
    mu sync.Mutex
}

Start Method with Idempotency Guard#

Start() prevents multiple concurrent goroutines:

func (s *SpinnerProgress) Start(message string) {
    s.mu.Lock()
    defer s.mu.Unlock()

    if s.running {
        return // Prevents multiple goroutines
    }

    s.message = message
    s.running = true
    s.done = make(chan struct{})
    s.stopped = make(chan struct{})

    go s.spin()
}

Stop Method with Write Safety Guarantee#

stop() waits for goroutine exit:

func (s *SpinnerProgress) stop() {
    s.mu.Lock()
    if !s.running {
        s.mu.Unlock()
        return
    }

    s.running = false
    close(s.done)
    stopped := s.stopped
    s.mu.Unlock()

    <-stopped // Wait for goroutine to exit
}

Spin Goroutine with Select#

spin() monitors done channel and signals completion:

func (s *SpinnerProgress) spin() {
    defer close(s.stopped)

    ticker := time.NewTicker(s.interval)
    defer ticker.Stop()

    for {
        select {
        case <-s.done:
            return
        case <-ticker.C:
            s.mu.Lock()
            s.current = (s.current + 1) % len(s.frames)
            frame := s.frames[s.current]
            msg := s.message
            s.mu.Unlock()

            fmt.Fprintf(s.output, "\r\033[K%s %s", frame, msg)
        }
    }
}

Key Safety Features:

• defer close(s.stopped) guarantees signal even on panic
• Select prioritizes done channel for responsive shutdown
• Mutex hold time minimized by copying data before I/O
• Tests verify idempotent stop behavior

sync.Once for Singleton Initialization#

The Pattern#

Go's sync.Once ensures that initialization code runs exactly once across all goroutines, with strict memory ordering guarantees from the Go memory model. This pattern is critical for lazy initialization of global singletons where multiple goroutines might race to initialize the same resource.

Memory Model Guarantee:
Per the Go memory model, sync.Once.Do(f) guarantees that all writes within function f happen-before any call to Do returns. This means all goroutines that observe the initialized singleton are guaranteed to see its fully initialized state without additional synchronization.

Global PluginRegistry Implementation (Deprecated)#

The global PluginRegistry singleton in internal/audit/plugin.go demonstrates this pattern, but is DEPRECATED and scheduled for removal in v2.0:

var (
    globalRegistry *PluginRegistry
    globalRegistryOnce sync.Once
)

// Deprecated: scheduled for removal in v2.0
func GetGlobalRegistry() *PluginRegistry {
    globalRegistryOnce.Do(func() {
        globalRegistry = NewPluginRegistry()
    })
    return globalRegistry
}

Key Properties:

• First caller executes initialization function and blocks
• Concurrent callers block until initialization completes
• Subsequent callers receive already-initialized instance with zero synchronization overhead
• All goroutines see consistent, fully-initialized state

Deprecation Notice:

• GetGlobalRegistry(), RegisterGlobalPlugin(), GetGlobalPlugin(), and ListGlobalPlugins() are DEPRECATED and scheduled for removal in v2.0
• These functions remain functional only for backward compatibility with legacy tests
• New production code must use NewPluginManager(logger, reg) with an explicit registry parameter

Lifecycle Contract: Register-at-Startup, Read-During-Operation#

The deprecated global registry followed a two-phase lifecycle pattern:

1. Initialization Phase (Sequential): All plugins register via RegisterGlobalPlugin() during application startup before concurrent access begins
2. Operation Phase (Concurrent Read-Only): Multiple goroutines safely read from the registry via GetGlobalRegistry()

This design pattern separated concerns:

• Sequential startup phase eliminated registration contention
• Runtime phase benefited from RWMutex read concurrency without write overhead
• Clear lifecycle boundaries simplified reasoning about thread safety

Note: This lifecycle contract is historical context. The pattern itself remains valid for registry instances, but the global singleton access functions are deprecated.

Registry Consolidation (Historical — Resolved)#

The global singleton registry was previously architecturally independent from PluginManager instances, creating silent "plugin registered in one, queried from the other" bugs. PluginManager.InitializePlugins() allocated and populated its own PluginRegistry, not the global singleton.

Historical Issue (Pre-2026-04-19):

• Calling pm.InitializePlugins() did not populate the global registry
• Plugins needed in both registries required explicit double registration
• This architectural independence led to subtle bugs

Resolved Architecture (2026-04-19):

• NewPluginManager(logger, reg) now accepts an explicit *PluginRegistry parameter
• Pass a shared *PluginRegistry when multiple managers or subsystems must observe the same plugin set
• Pass nil to allocate a fresh private registry (appropriate for short-lived callers)
• The previous split between PluginManager's private registry and the package-level global registry has been consolidated into a single registry path

Migration Guidance:

• Replace NewPluginManager(logger) with NewPluginManager(logger, nil) for isolated registry usage
• Replace NewPluginManager(logger) with NewPluginManager(logger, sharedReg) when registry sharing is required
• Do not call deprecated global registry functions (GetGlobalRegistry, RegisterGlobalPlugin, etc.)

See GOTCHAS §2.1 "Registry Consolidation (Historical — Resolved)" for additional context on this architectural change.

Context-Aware Patterns#

Context Underutilization#

The architecture review identifies inconsistent context usage in the current codebase:

Many functions accept context but don't utilize it

Current Pattern (context unused):

func (c *MarkdownConverter) ToMarkdown(_ context.Context, data *common.CommonDevice) (string, error)

Recommended Context-Aware Pattern#

For operations that should respect context cancellation and timeouts:

func (p *Processor) ProcessConfig(ctx context.Context, cfg *CommonDevice) error {
    // Check context before starting expensive work
    select {
    case <-ctx.Done():
        return ctx.Err()
    default:
    }

    for _, section := range cfg.Sections {
        select {
        case <-ctx.Done():
            return fmt.Errorf("processing cancelled: %w", ctx.Err())
        default:
        }

        if err := p.processSection(ctx, section); err != nil {
            return err
        }
    }

    return nil
}

Implementation Guidance:

• Check ctx.Done() at loop boundaries for long operations
• Use context.WithTimeout() for resource-intensive operations
• Propagate context through all called functions
• Return ctx.Err() for cancellation errors

Note: The internal/progress/ implementation uses done channels instead of context.Context, which is appropriate for the simpler spinner use case.

Sequential vs Concurrent Architecture#

Current Sequential Processing#

The architecture review notes sequential execution in the current implementation:

The current opnDossier implementation runs plugin checks and analysis phases sequentially

// Current: Sequential execution
for _, pluginName := range pluginNames {
    findings := p.RunChecks(config)
    result.Findings = append(result.Findings, findings...)
}

This is by design and appropriate for typical workloads.

Future Concurrent Patterns#

The architecture review recommends concurrent processing as future optimization:

"Add parallel processing for large-scale batch operations. Use worker pools to limit concurrency."

Worker Pool Pattern (recommended for future implementation):

func (p *Processor) ProcessWithWorkers(ctx context.Context, items []Item, workers int) error {
    taskChan := make(chan Item)
    errChan := make(chan error, workers)

    // Start worker goroutines
    var wg sync.WaitGroup
    for i := 0; i < workers; i++ {
        wg.Add(1)
        go func() {
            defer wg.Done()
            for {
                select {
                case task, ok := <-taskChan:
                    if !ok {
                        return
                    }
                    if err := p.processItem(ctx, task); err != nil {
                        select {
                        case errChan <- err:
                        case <-ctx.Done():
                        }
                    }
                case <-ctx.Done():
                    return
                }
            }
        }()
    }

    // Send tasks
    go func() {
        for _, item := range items {
            select {
            case taskChan <- item:
            case <-ctx.Done():
                close(taskChan)
                return
            }
        }
        close(taskChan)
    }()

    // Wait for completion
    go func() {
        wg.Wait()
        close(errChan)
    }()

    // Collect errors
    for err := range errChan {
        if err != nil {
            return err
        }
    }

    return nil
}

Usage Examples#

Example 1: Adding Thread-Safe Report Findings#

func (r *Report) AddFinding(finding Finding) {
    r.mu.Lock()
    defer r.mu.Unlock()

    switch finding.Severity {
    case "critical":
        r.Findings.Critical = append(r.Findings.Critical, finding)
    case "high":
        r.Findings.High = append(r.Findings.High, finding)
    // ... other cases
    }
}

Example 2: Reading Report Data Safely#

func (r *Report) HasCriticalFindings() bool {
    r.mu.RLock()
    defer r.mu.RUnlock()
    return len(r.Findings.Critical) > 0
}

Example 3: Starting and Stopping Progress Spinner#

spinner := progress.NewSpinner()
spinner.Start("Processing configuration...")

// Do work
time.Sleep(2 * time.Second)

spinner.Complete("Done!") // Safely stops goroutine and writes completion message

Example 4: Creating Unsafe Helper for Internal Use#

// Public method - acquires lock
func (r *Registry) ValidatePlugin(name string) error {
    r.mu.RLock()
    defer r.mu.RUnlock()
    return r.validatePluginUnsafe(name)
}

// Internal unsafe method - assumes lock is held
// Caller must hold mu for reading
func (r *Registry) validatePluginUnsafe(name string) error {
    plugin, ok := r.plugins[name]
    if !ok {
        return fmt.Errorf("plugin not found: %s", name)
    }
    return plugin.Validate()
}

Relevant Code Files#

Design Principles#

1. Prefer Sequential When Possible - Simpler reasoning, fewer bugs
2. Use sync.RWMutex for Read-Heavy Workloads - Plugin registry, report reading
3. Use sync.Once for Singleton Initialization - Lazy initialization with memory model guarantees
4. Implement Unsafe Helpers for Non-Reentrant Locks - Avoid deadlocks
5. Return Value Copies from Locked Sections - Prevent data races
6. Use Stopped Channel for Goroutine Write Safety - Coordinate shutdown
7. Document Lock Assumptions - Unsafe methods need clear documentation
8. Test Concurrent Access - Verify no data races with -race flag
9. Minimize Mutex Hold Time - Copy data before expensive operations

Related Topics#

• Plugin Architecture: The plugin registry uses RWMutex for thread-safe plugin registration and access
• Error Handling: Goroutine error propagation through channels
• Testing: Race detection with go test -race
• Context Package: Standard Go context for cancellation and timeouts
• Sync Package: Go standard library primitives (RWMutex, Mutex, WaitGroup)

References#

• Architecture Review - Concurrency Considerations
• Development Standards - Thread Safety
• Task Documentation - Future Concurrency
• Go Blog: Share Memory By Communicating
• Go Effective: Concurrency