Fix #4715: Add mutex protection to export.Manager (#4738)

This commit is contained in:
Nathan Wallace
2025-11-11 18:54:52 +08:00
committed by GitHub
parent 7996d3c878
commit a7cb5caa02
2 changed files with 68 additions and 0 deletions

View File

@@ -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,

View File

@@ -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)
}
}
}