* Add test for #4809: BeforeClose should handle nil sessions map
This test verifies that the BeforeClose callback checks if c.sessions map
has been nil'd by Close() before attempting to delete from it.
The test fails as expected without the fix, proving the bug exists.
Bug: #4809🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix#4809: Add nil check in BeforeClose for sessions map
Add nil check in BeforeClose callback before accessing c.sessions map.
This prevents panic when the callback executes after Close() has nil'd
the map.
Fixes#4809🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Fixes two flaky tests that were failing intermittently in CI:
- TestPluginManager_ConcurrentUpdateRateLimiterStatus
- TestPluginManager_ConcurrentRateLimiterMapAccess2
Root cause: The test writer goroutines were modifying pm.userLimiters
without mutex protection, while reader goroutines were simultaneously
accessing the map. Go's map implementation detects concurrent read/write
access and panics with "fatal error: concurrent map read and map write".
The production code in handleUserLimiterChanges (lines 98-100) correctly
protects writes with pm.mut.Lock/Unlock. The tests needed to simulate
this same behavior.
Changes:
- Added missing mut field initialization in TestPluginManager_ConcurrentUpdateRateLimiterStatus
- Wrapped all pm.userLimiters writes in both tests with pm.mut.Lock/Unlock
- Added comments explaining the mutex usage matches production code
Testing: Ran tests 100+ times consecutively with and without -race flag,
all passed successfully. Previously failed on 2nd iteration without the fix.
* 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>
* Add test for #4799: Race condition in rate limiter map access
This test demonstrates the race condition in getUserDefinedLimitersForPlugin
where the userLimiters map is read without mutex protection while being
concurrently written by other goroutines.
Run with: go test -race -v -run TestPluginManager_ConcurrentRateLimiterMapAccess ./pkg/pluginmanager_service
* Fix#4799: Add mutex protection for rate limiter map access
Protected all accesses to m.userLimiters map with RWMutex:
- getUserDefinedLimitersForPlugin: Added RLock for map read
- getPluginsWithChangedLimiters: Added RLock for map iteration
- handleUserLimiterChanges: Added Lock for map write
- refreshRateLimiterTable: Added RLock for map iteration
- setRateLimiters: Added RLock for map read
This prevents data races when the map is concurrently read and written
by multiple goroutines.
* Fix inconsistent value for plugin in steampipe_plugin_column table - always use long name.
* Rename `plugin_name` column to `plugin`
* Ensure empty col values
* Update PopulateChildren.PopulateChildren to take plugin instance into account
* Fix GetNewConnectionStateFromConnectionInsertSql to use connection names not map of connections
