Skip to content

[data race] Mock Verifier Updates Slice #452

@liran-funaro

Description

@liran-funaro

This is an apparent race that appears only in tests. Not a real issue.

Mock Verifier Updates Slice

Location

  • Read: service/coordinator/signature_verifier_manager_test.go:477 (test goroutine)
  • Write: mock/sigverifier.go:103 (gRPC stream goroutine)

Root Cause

The VerifierStreamState.Updates slice is accessed concurrently without synchronization:

Writer (mock/sigverifier.go:103):

func (m *VerifierStreamState) updatePolicies(update *servicepb.VerifierUpdates) error {
    // ...
    m.Updates = append(m.Updates, update)  // ← WRITE (line 103)
    // ...
}

Reader (signature_verifier_manager_test.go:477):

func (e *svMgrTestEnv) requireAllUpdate(...) {
    require.Eventually(t, func() bool {
        for i, sv := range svs {
            u := sv.Updates  // ← READ (line 477)
            if len(u) < expectedCount {
                // ...
            }
        }
    }, ...)
}

Problem

  1. The updatePolicies() method runs in a gRPC stream goroutine (goroutine 1602)
  2. The test's requireAllUpdate() polls Updates from a different goroutine (goroutine 1640)
  3. Go slices are not thread-safe - concurrent read/append operations cause data races
  4. The append() operation can trigger slice reallocation, making concurrent reads particularly dangerous

Impact

  • Test flakiness and potential crashes
  • Undefined behavior when slice is reallocated during concurrent read
  • Race detector failures in CI/CD

Fix: Mock Verifier Updates

Option A - Use mutex:

type VerifierStreamState struct {
    // ...
    updatesMu sync.RWMutex
    Updates   []*servicepb.VerifierUpdates
}

func (m *VerifierStreamState) updatePolicies(update *servicepb.VerifierUpdates) error {
    m.updatesMu.Lock()
    defer m.updatesMu.Unlock()
    m.Updates = append(m.Updates, update)
    // ...
}

// In test:
sv.updatesMu.RLock()
u := sv.Updates
sv.updatesMu.RUnlock()

Option B - Use atomic.Value (if only reading in tests):

type VerifierStreamState struct {
    updates atomic.Value // []*servicepb.VerifierUpdates
}


 ==================
  WARNING: DATA RACE
  Read at 0x00c003ad66c0 by goroutine 1640:
    github.com/hyperledger/fabric-x-committer/service/coordinator.(*svMgrTestEnv).requireAllUpdate.func1()
        /home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/signature_verifier_manager_test.go:477 +0x22a
    github.com/stretchr/testify/assert.Eventually.func1()
        /home/runner/go/pkg/mod/github.com/stretchr/testify@v1.11.1/assert/assertions.go:1994 +0x33
  
  Previous write at 0x00c003ad66c0 by goroutine 1602:
    github.com/hyperledger/fabric-x-committer/mock.(*VerifierStreamState).updatePolicies()
        /home/runner/work/fabric-x-committer/fabric-x-committer/mock/sigverifier.go:103 +0x1ab
    github.com/hyperledger/fabric-x-committer/mock.(*VerifierStreamState).receiveRequestBatch()
        /home/runner/work/fabric-x-committer/fabric-x-committer/mock/sigverifier.go:119 +0x2b0
    github.com/hyperledger/fabric-x-committer/mock.(*Verifier).StartStream.func2()
        /home/runner/work/fabric-x-committer/fabric-x-committer/mock/sigverifier.go:82 +0x77
    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 1640 (running) created at:
    github.com/stretchr/testify/assert.Eventually()
        /home/runner/go/pkg/mod/github.com/stretchr/testify@v1.11.1/assert/assertions.go:2013 +0x39d
    github.com/stretchr/testify/require.Eventually()
        /home/runner/go/pkg/mod/github.com/stretchr/testify@v1.11.1/require/require.go:412 +0xd1
    github.com/hyperledger/fabric-x-committer/service/coordinator.(*svMgrTestEnv).requireAllUpdate()
        /home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/signature_verifier_manager_test.go:462 +0x20d
    github.com/hyperledger/fabric-x-committer/service/coordinator.TestSignatureVerifierManagerPolicyUpdateAndRecover()
        /home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/signature_verifier_manager_test.go:381 +0x60a
    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 1602 (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/mock.(*Verifier).StartStream()
        /home/runner/work/fabric-x-committer/fabric-x-committer/mock/sigverifier.go:81 +0x277
    github.com/hyperledger/fabric-x-committer/api/servicepb._Verifier_StartStream_Handler()
        /home/runner/work/fabric-x-committer/fabric-x-committer/api/servicepb/verifier_grpc.pb.go:99 +0x13e
    google.golang.org/grpc.(*Server).processStreamingRPC()
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.78.0/server.go:1714 +0x1d7c
    google.golang.org/grpc.(*Server).handleStream()
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.78.0/server.go:1836 +0x16d2
    google.golang.org/grpc.(*Server).serveStreams.func2.1()
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.78.0/server.go:1063 +0x149
  ==================

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions