-
Notifications
You must be signed in to change notification settings - Fork 684
Description
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.