-
Notifications
You must be signed in to change notification settings - Fork 13
[data race] SimpleManager #453
Copy link
Copy link
Labels
Description
This is an apparent race that appears only in tests. Not a real issue.
SimpleManager waitingTXs Counter
Location
- Read:
service/coordinator/dependencygraph/manager_test.go:382(test goroutine) - Write:
service/coordinator/dependencygraph/simple.go:178(processing goroutine)
Root Cause
The SimpleManager.waitingTXs integer field is accessed concurrently without synchronization:
Writer (simple.go:178):
func (m *SimpleManager) processValidatedBatch(val validatedBatch) TxNodeBatch {
m.waitingTXs -= val.txCount // ← WRITE (line 178)
// ...
}Reader (manager_test.go:382):
func getWaitingTXs(t *testing.T, m any) int {
switch mt := m.(type) {
case *SimpleManager:
return mt.waitingTXs // ← READ (line 382)
// ...
}
}Problem
- The
processValidatedBatch()method runs in thetaskProcessing()goroutine (goroutine 215) - The test's
ensureWaitingTXsLimit()pollswaitingTXsviarequire.EventuallyWithT()(goroutine 587) - Plain integer reads/writes are not atomic in Go - concurrent access without synchronization is a data race
- The field is also modified in
processTxBatch()(line 156:m.waitingTXs += len(batch))
Impact
- Test flakiness with incorrect values read
- Race detector failures
- Potential for reading partially written values on some architectures
Fix: SimpleManager waitingTXs
Use atomic operations:
type SimpleManager struct {
// ...
waitingTXs atomic.Int32 // Change from int to atomic.Int32
}
// In production code:
m.waitingTXs.Add(-int32(val.txCount))
m.waitingTXs.Add(int32(len(batch)))
// In test code:
return int(mt.waitingTXs.Load())==================
WARNING: DATA RACE
Read at 0x00c000475528 by goroutine 587:
github.com/hyperledger/fabric-x-committer/service/coordinator/dependencygraph.getWaitingTXs()
/home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/dependencygraph/manager_test.go:382 +0x78
github.com/hyperledger/fabric-x-committer/service/coordinator/dependencygraph.ensureWaitingTXsLimit.func1()
/home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/dependencygraph/manager_test.go:364 +0x5e
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 0x00c000475528 by goroutine 215:
github.com/hyperledger/fabric-x-committer/service/coordinator/dependencygraph.(*SimpleManager).processValidatedBatch()
/home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/dependencygraph/simple.go:178 +0xc4
github.com/hyperledger/fabric-x-committer/service/coordinator/dependencygraph.(*SimpleManager).taskProcessing()
/home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/dependencygraph/simple.go:144 +0x5ca
github.com/hyperledger/fabric-x-committer/service/coordinator/dependencygraph.(*SimpleManager).Run.func3()
/home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/dependencygraph/simple.go:79 +0xad
Goroutine 587 (running) created at:
github.com/stretchr/testify/assert.EventuallyWithT()
/home/runner/go/pkg/mod/github.com/stretchr/testify@v1.11.1/assert/assertions.go:2108 +0x278
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/service/coordinator/dependencygraph.ensureWaitingTXsLimit()
/home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/dependencygraph/manager_test.go:363 +0x12a
github.com/hyperledger/fabric-x-committer/service/coordinator/dependencygraph.TestDependencyGraphManager.func2()
/home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/dependencygraph/manager_test.go:356 +0x30a8
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 215 (running) created at:
github.com/hyperledger/fabric-x-committer/service/coordinator/dependencygraph.(*SimpleManager).Run()
/home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/dependencygraph/simple.go:77 +0x3a4
github.com/hyperledger/fabric-x-committer/service/coordinator/dependencygraph.startManager.func1()
/home/runner/work/fabric-x-committer/fabric-x-committer/service/coordinator/dependencygraph/manager_test.go:417 +0x50
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
==================
Reactions are currently unavailable