* Unskip test demonstrating bug #4805: Concurrent read and close may race
Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix#4805: Add synchronization for concurrent StreamRow and Close
The sync.Once in Close() only prevents multiple Close() calls, but
doesn't coordinate with StreamRow() operations. Added a mutex and
closed flag to prevent race conditions when one goroutine streams
rows while another closes the result.
The fix:
- Added mutex (mu) and closed flag to Result struct
- StreamRow checks closed flag before streaming (with RLock)
- Close sets closed flag (with Lock) before closing channel
This prevents "send on closed channel" panics and data races.
Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
* Add test for #4803: Race condition in initialisationComplete flag
Add TestInitialisationComplete_RaceCondition to demonstrate the data race
that occurs when the initialisationComplete boolean flag is accessed
concurrently by multiple goroutines without synchronization.
The test simulates:
- Init goroutine writing to the flag
- Query executor reading via isInitialised()
- Notification handler reading the flag directly
This test will fail when run with the -race flag, exposing the bug.
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix#4803: Use atomic.Bool for initialisationComplete flag
Replace the plain boolean initialisationComplete field with atomic.Bool
to prevent data races when accessed concurrently by multiple goroutines.
Changes:
- Change field type from bool to atomic.Bool
- Use .Store(true) for writes
- Use .Load() for reads in isInitialised() and handleConnectionUpdateNotification()
- Update test to use atomic operations
The test now passes with -race flag, confirming the race condition is fixed.
---------
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.
* Add test for #4814: Logic error in IdentifyMissingComments
Demonstrates the bug where the function uses OR (||) instead of AND (&&)
on line 426, causing connections being deleted to be incorrectly added
to MissingComments.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix#4814: Change OR to AND in IdentifyMissingComments logic
Corrects the logic error on line 426 where OR (||) was incorrectly used
instead of AND (&&). The condition should be "if NOT updating AND NOT
deleting" to properly exclude connections being updated or deleted from
the MissingComments list.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Changes:
1. Renamed TestQueryContextLeakage -> TestContextCancellationTiming
- The test was checking cancellation timing, not memory leaks
- Updated comments to reflect actual purpose
2. Increased timeout from 1ms to 100ms
- 1ms is too aggressive for CI runners under load
- 100ms still catches real deadlocks while avoiding flakiness
- Added detailed comments explaining the timeout choice
3. Added TestNoGoroutineLeaks using goleak
- Properly tests for actual resource leaks (goroutines)
- More reliable than memory-based leak detection
- Uses industry-standard goleak library
The original 1ms timeout caused intermittent CI failures on slower
runners, as context cancellation involves goroutine scheduling that
has no guaranteed sub-millisecond timing.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude <noreply@anthropic.com>
