[data-race] TestLoadGenRateLimiterServer

[liran-funaro]

In the test TestLoadGenRateLimiterServer we wait for the server to obtain a port with require.EventuallyWithT().
This is an apparent race. Not a real issue.

---

Summary

This is a data race in TestLoadGenRateLimiterServer where the test goroutine reads Endpoint.Port while the HTTP server goroutine concurrently writes to it during initialization.

Root Cause Analysis

Race Condition Details:

• Read location: loadgen/client_test.go:473 - Test polling rlEndpoint.Port
• Write location: utils/connection/server_util.go:120 - ServerConfig.Listener() writing to c.Endpoint.Port

Critical Timing Issue:

1. Test starts client with test.RunServiceForTest() → calls client.Run()
2. client.Run() signals ready at line ~130 BEFORE HTTP server initializes
3. HTTP server runs in goroutine (line ~124) and calls Listener() at line ~212
4. Test's WaitForReady() returns, then test immediately polls rlEndpoint.Port
5. Race: Test reads Port while HTTP server goroutine writes to it

Why WaitForReady Doesn't Help:
The ready signal is sent before the HTTP server's Listener() call, so WaitForReady() returns before the port is assigned.

Recommended Solution

Use PreAllocateListener() (Best Option)

Pre-allocate the listener before creating the client to bind the port synchronously:

func TestLoadGenRateLimiterServer(t *testing.T) {
    t.Parallel()
    clientConf := DefaultClientConf(t, test.InsecureTLSConfig)
    clientConf.Adapter.VerifierClient = startVerifiers(t, test.InsecureTLSConfig, test.InsecureTLSConfig)
    curRate := uint64(100)
    clientConf.Stream.RateLimit = curRate
    clientConf.LoadProfile.Block.PreferredRate = 10 * time.Millisecond
    clientConf.LoadProfile.Block.MaxSize = 100
    clientConf.LoadProfile.Block.MinSize = 1
    clientConf.HTTPServer = connection.NewLocalHostServer(test.InsecureTLSConfig)
    
    // Pre-allocate listener to bind port synchronously
    _, err := clientConf.HTTPServer.PreAllocateListener()
    require.NoError(t, err)
    
    client, err := NewLoadGenClient(clientConf)
    require.NoError(t, err)

    test.RunServiceForTest(t.Context(), t, client.Run, client.WaitForReady)
    
    // Port is now safely assigned - no race
    rlEndpoint := &clientConf.HTTPServer.Endpoint
    require.NotZero(t, rlEndpoint.Port)
    t.Logf("limiter endpoint: %v", rlEndpoint)
    
    // ... rest of test
}

Why This Works:

• PreAllocateListener() calls Listener() synchronously in the test goroutine
• Port is assigned before client.Run() starts
• HTTP server's Listener() call reuses the pre-allocated listener (no write to Port)
• No concurrent access to Endpoint.Port

Alternative Solutions (Not Recommended)

Option 2: Add synchronization to ServerConfig
Add mutex protection to Endpoint.Port - too invasive for a test-only issue.

Option 3: Copy endpoint value
Still has a race during the copy operation.

Priority

Minor - Test-only issue, but should be fixed to maintain clean CI output and demonstrate proper concurrent programming.

Implementation

The fix is simple, low-risk, and uses existing infrastructure (PreAllocateListener() already exists for this purpose).

---

• detected by [CI] Enable race detection for unit tests in CI #442

 ==================
  WARNING: DATA RACE
  Read at 0x00c000422be0 by goroutine 621:
    github.com/hyperledger/fabric-x-committer/loadgen.TestLoadGenRateLimiterServer.func1()
        /home/runner/work/fabric-x-committer/fabric-x-committer/loadgen/client_test.go:473 +0x35
    github.com/stretchr/testify/assert.EventuallyWithT.func1()
        /home/runner/go/pkg/mod/github.com/stretchr/testify@v1.11.1/assert/assertions.go:2096 +0xb3
  
  Previous write at 0x00c000422be0 by goroutine 401:
    github.com/hyperledger/fabric-x-committer/utils/connection.(*ServerConfig).Listener()
        /home/runner/work/fabric-x-committer/fabric-x-committer/utils/connection/server_util.go:120 +0x2b8
    github.com/hyperledger/fabric-x-committer/loadgen.(*Client).runHTTPServer()
        /home/runner/work/fabric-x-committer/fabric-x-committer/loadgen/client.go:212 +0x433
    github.com/hyperledger/fabric-x-committer/loadgen.(*Client).Run.func2()
        /home/runner/work/fabric-x-committer/fabric-x-committer/loadgen/client.go:124 +0x58
    golang.org/x/sync/errgroup.(*Group).Go.func1()
        /home/runner/go/pkg/mod/golang.org/x/sync@v0.19.0/errgroup/errgroup.go:93 +0x86
  
  Goroutine 621 (running) created at:
    github.com/stretchr/testify/assert.EventuallyWithT()
        /home/runner/go/pkg/mod/github.com/stretchr/testify@v1.11.1/assert/assertions.go:2119 +0x3a4
    github.com/stretchr/testify/require.EventuallyWithT()
        /home/runner/go/pkg/mod/github.com/stretchr/testify@v1.11.1/require/require.go:440 +0xd1
    github.com/hyperledger/fabric-x-committer/loadgen.TestLoadGenRateLimiterServer()
        /home/runner/work/fabric-x-committer/fabric-x-committer/loadgen/client_test.go:472 +0x8d1
    testing.tRunner()
        /opt/hostedtoolcache/go/1.26.1/x64/src/testing/testing.go:2036 +0x21c
    testing.(*T).Run.gowrap1()
        /opt/hostedtoolcache/go/1.26.1/x64/src/testing/testing.go:2101 +0x38
  
  Goroutine 401 (running) created at:
    golang.org/x/sync/errgroup.(*Group).Go()
        /home/runner/go/pkg/mod/golang.org/x/sync@v0.19.0/errgroup/errgroup.go:78 +0x11c
    github.com/hyperledger/fabric-x-committer/loadgen.(*Client).Run()
        /home/runner/work/fabric-x-committer/fabric-x-committer/loadgen/client.go:123 +0x424
    github.com/hyperledger/fabric-x-committer/loadgen.(*Client).Run-fm()
        <autogenerated>:1 +0x3e
    github.com/hyperledger/fabric-x-committer/utils/test.RunServiceForTest.func1()
        /home/runner/work/fabric-x-committer/fabric-x-committer/utils/test/utils.go:205 +0x11e
    sync.(*WaitGroup).Go.func1()
        /opt/hostedtoolcache/go/1.26.1/x64/src/sync/waitgroup.go:258 +0x5d
  ==================