Skip to content

CListMempool has data race(s) #642

@peterbourgon

Description

@peterbourgon

mempool.CListMempool.Update writes to the height field under the assumption that the updateMtx field on the CListMempool struct has been locked by its caller. But e.g. mempool.CListMempool.resCbFirstTime reads from the height field without taking that lock. And that method is called by reqResCb via abci/client.ReqRes.SetCallback, neither of which take the lock. This is a data race.

The height field (and others) are commented as "atomic" which suggests they are expected to be accessed with sync/atomic functions like mempoolTx.Height. But that's not the case at the moment. If that's the intent, then they should only be accessed with sync/atomic functions, and do not require the updateMtx to be locked. But this kind of mixing of individually-atomic fields and a mutex that governs (apparently arbitrary) fields in the type is basically impossible to get right :) and probably deserves a refactor.

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingmempool

Type

No type

Projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions