Race condition in updateRateLimiterStatus closes #4786 (#4830)

* Add test demonstrating bug #4786 - race in updateRateLimiterStatus

Test demonstrates the race condition when updateRateLimiterStatus reads
from the userLimiters map without holding a lock, while another goroutine
concurrently writes to the map.

Running with -race flag shows data races at:
- plugin_manager_rate_limiters.go:176 (read in getUserDefinedLimitersForPlugin)
- plugin_manager_rate_limiters.go:161 (read in updateRateLimiterStatus)
- rate_limiters_test.go:40 (concurrent write)

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix #4786: Protect updateRateLimiterStatus with RWMutex

Add RWMutex write lock protection to updateRateLimiterStatus to prevent
race conditions when the method reads from userLimiters map and writes to
pluginLimiter.Status fields while other goroutines concurrently modify these
data structures.

The fix uses m.mut.Lock() (not RLock) because the method modifies the
pluginLimiter.Status field, requiring exclusive write access.

Note: This fix assumes updateRateLimiterStatus is not called from within
a context that already holds the mutex. If it is, additional refactoring
will be needed to prevent deadlock.

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix deadlock in updateRateLimiterStatus

The previous fix introduced a deadlock where updateRateLimiterStatus() would:
1. Acquire m.mut.Lock()
2. Call getUserDefinedLimitersForPlugin()
3. Which tries to acquire m.mut.RLock() - deadlock!

Go mutexes cannot acquire RLock when the same goroutine already holds Lock.

Fix by refactoring into internal/public pattern:
- getUserDefinedLimitersForPlugin() - public, acquires RLock
- getUserDefinedLimitersForPluginInternal() - internal, no lock (caller must hold it)
- updateRateLimiterStatus() now calls internal version while holding lock

Test verification: TestPluginManager_UpdateRateLimiterStatus_NoOverride
- Before: Timeout after 28s
- After: Pass in 0.429s

Fixes #4786

---------

Co-authored-by: Claude <noreply@anthropic.com>

pkg/pluginmanager_service/plugin_manager_rate_limiters.go

@@ -162,10 +162,13 @@ func (m *PluginManager) getPluginsWithChangedLimiters(newLimiters connection.Plu
 }
 
 func (m *PluginManager) updateRateLimiterStatus() {
+	m.mut.Lock()
+	defer m.mut.Unlock()
+
 	// iterate through limiters for each plug
 	for p, pluginDefinedLimiters := range m.pluginLimiters {
-		// get user limiters for this plugin
-		userDefinedLimiters := m.getUserDefinedLimitersForPlugin(p)
+		// get user limiters for this plugin (already holding lock, so call internal version)
+		userDefinedLimiters := m.getUserDefinedLimitersForPluginInternal(p)
 
 		// is there a user override? - if so set status to overriden
 		for name, pluginLimiter := range pluginDefinedLimiters {
@@ -182,6 +185,12 @@ func (m *PluginManager) updateRateLimiterStatus() {
 func (m *PluginManager) getUserDefinedLimitersForPlugin(plugin string) connection.LimiterMap {
 	m.mut.RLock()
 	defer m.mut.RUnlock()
+	return m.getUserDefinedLimitersForPluginInternal(plugin)
+}
+
+// getUserDefinedLimitersForPluginInternal returns user-defined limiters for a plugin
+// WITHOUT acquiring the lock - caller must hold the lock
+func (m *PluginManager) getUserDefinedLimitersForPluginInternal(plugin string) connection.LimiterMap {
 	userDefinedLimiters := m.userLimiters[plugin]
 	if userDefinedLimiters == nil {
 		userDefinedLimiters = make(connection.LimiterMap)

pkg/pluginmanager_service/rate_limiters_test.go

@@ -91,3 +91,112 @@ func TestPluginManager_ConcurrentRateLimiterMapAccess(t *testing.T) {
 		t.Error("Expected userLimiters to be non-nil")
 	}
 }
+
+// TestPluginManager_ConcurrentUpdateRateLimiterStatus tests for race condition
+// when updateRateLimiterStatus is called concurrently with writes to userLimiters map
+// References: https://github.com/turbot/steampipe/issues/4786
+func TestPluginManager_ConcurrentUpdateRateLimiterStatus(t *testing.T) {
+	// Create a PluginManager with test data
+	pm := &PluginManager{
+		userLimiters: make(connection.PluginLimiterMap),
+		pluginLimiters: connection.PluginLimiterMap{
+			"aws": connection.LimiterMap{
+				"limiter1": &plugin.RateLimiter{
+					Name:   "limiter1",
+					Plugin: "aws",
+					Status: plugin.LimiterStatusActive,
+				},
+			},
+		},
+	}
+
+	// Run concurrent operations to trigger race condition
+	var wg sync.WaitGroup
+	iterations := 100
+
+	// Writer goroutine - simulates handleUserLimiterChanges modifying userLimiters
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		for i := 0; i < iterations; i++ {
+			pm.userLimiters = connection.PluginLimiterMap{
+				"aws": connection.LimiterMap{
+					"limiter1": &plugin.RateLimiter{
+						Name:   "limiter1",
+						Plugin: "aws",
+						Status: plugin.LimiterStatusOverridden,
+					},
+				},
+			}
+		}
+	}()
+
+	// Reader goroutine - simulates updateRateLimiterStatus reading userLimiters
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		for i := 0; i < iterations; i++ {
+			pm.updateRateLimiterStatus()
+		}
+	}()
+
+	wg.Wait()
+}
+
+// TestPluginManager_ConcurrentRateLimiterMapAccess2 tests for race condition
+// when multiple goroutines access pluginLimiters and userLimiters concurrently
+func TestPluginManager_ConcurrentRateLimiterMapAccess2(t *testing.T) {
+	pm := &PluginManager{
+		userLimiters: connection.PluginLimiterMap{
+			"aws": connection.LimiterMap{
+				"limiter1": &plugin.RateLimiter{
+					Name:   "limiter1",
+					Plugin: "aws",
+					Status: plugin.LimiterStatusOverridden,
+				},
+			},
+		},
+		pluginLimiters: connection.PluginLimiterMap{
+			"aws": connection.LimiterMap{
+				"limiter1": &plugin.RateLimiter{
+					Name:   "limiter1",
+					Plugin: "aws",
+					Status: plugin.LimiterStatusActive,
+				},
+			},
+		},
+	}
+
+	var wg sync.WaitGroup
+	iterations := 50
+
+	// Multiple readers
+	for i := 0; i < 3; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for j := 0; j < iterations; j++ {
+				pm.updateRateLimiterStatus()
+			}
+		}()
+	}
+
+	// Multiple writers
+	for i := 0; i < 2; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for j := 0; j < iterations; j++ {
+				pm.userLimiters["aws"] = connection.LimiterMap{
+					"limiter1": &plugin.RateLimiter{
+						Name:   "limiter1",
+						Plugin: "aws",
+						Status: plugin.LimiterStatusOverridden,
+					},
+				}
+			}
+		}()
+	}
+
+	wg.Wait()
+}