mirror of
https://github.com/mattermost/mattermost.git
synced 2026-10-29 18:53:04 +08:00
These three tests have been failing deterministically on every master build since Go 1.25.8 was merged (PR #35817, March 27). The failures only manifest on master because PR CI does not run with the -race flag. ## Root Cause The race is between Go's database/sql connection pool and testify's mock assertion logic: 1. When a *sql.DB connection pool is opened, Go spawns a background connectionCleaner goroutine that periodically runs connectionCleanerRunLocked(), which acquires the DB's internal mutex via atomic.CompareAndSwapInt32. 2. The tests pass a *sql.DB argument to mock expectations via RegisterDBCollector(). When testify's AssertExpectations() runs, it uses reflect to diff the recorded call arguments, which reads the internal fields of the *sql.DB struct (including the mutex state and connection pool fields) via reflect.Value.Int() and reflect.typedmemmove(). 3. Go 1.25 ships a stricter race detector that catches this concurrent read (reflect in the test goroutine) vs write (atomic CAS in the connectionCleaner goroutine) on the same memory. Go 1.24's race detector did not flag this pattern. The race existed latently in the code for a long time. Go 1.25 simply made it detectable. ## Fix Close/shut down the store (and all its background goroutines, including the sql.DB connection cleaner) BEFORE calling AssertExpectations. This ensures there is no concurrent writer when testify uses reflect to inspect the *sql.DB arguments. - TestReplicaLagQuery: move store.Close() before AssertExpectations (was previously deferred, running after AssertExpectations) - TestInvalidReplicaLagDataSource: replace defer store.Close() with explicit store.Close() at end of test - TestMetrics/ensure_advanced_metrics_have_database_metrics: call th.Shutdown(t) before AssertExpectations (Shutdown was previously registered via t.Cleanup, running after the test function returns) Co-authored-by: Claude <claude@anthropic.com>