From a7cb5caa022e52231de98d1239c6b38826e6b222 Mon Sep 17 00:00:00 2001 From: Nathan Wallace Date: Tue, 11 Nov 2025 18:54:52 +0800 Subject: [PATCH] Fix #4715: Add mutex protection to export.Manager (#4738) --- pkg/export/manager.go | 8 +++++ pkg/export/manager_test.go | 60 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/pkg/export/manager.go b/pkg/export/manager.go index 0dae1d0d2..927c3fca1 100644 --- a/pkg/export/manager.go +++ b/pkg/export/manager.go @@ -5,6 +5,7 @@ import ( "fmt" "path" "strings" + "sync" "github.com/turbot/pipe-fittings/v2/utils" "github.com/turbot/steampipe-plugin-sdk/v5/sperr" @@ -17,6 +18,7 @@ import ( type Manager struct { registeredExporters map[string]Exporter registeredExtensions map[string]Exporter + mu sync.RWMutex } func NewManager() *Manager { @@ -27,6 +29,9 @@ func NewManager() *Manager { } func (m *Manager) Register(exporter Exporter) error { + m.mu.Lock() + defer m.mu.Unlock() + name := exporter.Name() if _, ok := m.registeredExporters[name]; ok { return fmt.Errorf("failed to register exporter - duplicate name %s", name) @@ -114,6 +119,9 @@ func (m *Manager) resolveTargetsFromArgs(exportArgs []string, executionName stri } func (m *Manager) getExportTarget(export, executionName string) (*Target, error) { + m.mu.RLock() + defer m.mu.RUnlock() + if e, ok := m.registeredExporters[export]; ok { t := &Target{ exporter: e, diff --git a/pkg/export/manager_test.go b/pkg/export/manager_test.go index f921756ba..132e7b788 100644 --- a/pkg/export/manager_test.go +++ b/pkg/export/manager_test.go @@ -132,3 +132,63 @@ func TestDoExport(t *testing.T) { } } } + +// TestManager_ConcurrentRegistration tests that the Manager can handle concurrent +// exporter registration safely. This test is designed to expose race conditions +// when run with the -race flag. +// +// Related issue: #4715 +func TestManager_ConcurrentRegistration(t *testing.T) { + // Create a manager instance + m := NewManager() + + // Create multiple test exporters with unique names + exporters := []*testExporter{ + {alias: "", extension: ".csv", name: "csv"}, + {alias: "", extension: ".json", name: "json"}, + {alias: "", extension: ".xml", name: "xml"}, + {alias: "", extension: ".html", name: "html"}, + {alias: "", extension: ".yaml", name: "yaml"}, + {alias: "", extension: ".md", name: "markdown"}, + {alias: "", extension: ".txt", name: "text"}, + {alias: "", extension: ".log", name: "log"}, + } + + // Channel to collect errors from goroutines + errChan := make(chan error, len(exporters)) + done := make(chan bool) + + // Register all exporters concurrently + for _, exp := range exporters { + go func(e *testExporter) { + err := m.Register(e) + errChan <- err + }(exp) + } + + // Collect results + go func() { + for i := 0; i < len(exporters); i++ { + err := <-errChan + if err != nil { + t.Errorf("Failed to register exporter: %v", err) + } + } + done <- true + }() + + // Wait for completion + <-done + + // Verify all exporters were registered successfully + // Each exporter should be accessible by its name + for _, exp := range exporters { + target, err := m.getExportTarget(exp.name, "test_exec") + if err != nil { + t.Errorf("Exporter '%s' was not registered properly: %v", exp.name, err) + } + if target == nil { + t.Errorf("Exporter '%s' returned nil target", exp.name) + } + } +}