Summary
Several package-level globals in walletapi are written and read from multiple goroutines with no synchronization:
|
var Connected bool = false |
|
|
|
var daemon_height int64 |
|
var daemon_topoheight int64 |
Connected (bool) — written by Connect, test_connectivity, Keep_Connectivity; read by IsDaemonOnlineCached and consumers.daemon_height / daemon_topoheight (int64) — written by test_connectivity (notification goroutine) and GetEncryptedBalanceAtTopoHeight (sync loop / RPC handler goroutines); read by Get_Daemon_Height, transfer building (daemon_topoheight at walletapi/wallet_transfer.go:238), history sync, and event dispatch.simulator (bool, line 76) — written by test_connectivity, read during fee construction.
Impact
These are data races under the Go memory model — go run -race / go test -race flags them immediately. Practical symptoms are stale or torn reads on 32-bit platforms and reordering surprises (e.g. transfer building reading a daemon_topoheight that's inconsistent with the balance snapshot it pairs with). It also blocks ever running the race detector cleanly in CI, which hides new races (see the discussion on PR #26).
Fix
Short term: switch to sync/atomic types (atomic.Int64, atomic.Bool) or guard with a small mutex.
Long term: the file's own comment (line 74: "there should be no global variables, so multiple wallets can run at the same time") already points the way — move connection state into a client struct so multiple wallets/daemon connections can coexist in one process.
Summary
Several package-level globals in
walletapiare written and read from multiple goroutines with no synchronization:derohe/walletapi/daemon_communication.go
Lines 60 to 63 in 6f9bd2c
Connected(bool) — written byConnect,test_connectivity,Keep_Connectivity; read byIsDaemonOnlineCachedand consumers.daemon_height/daemon_topoheight(int64) — written bytest_connectivity(notification goroutine) andGetEncryptedBalanceAtTopoHeight(sync loop / RPC handler goroutines); read byGet_Daemon_Height, transfer building (daemon_topoheightat walletapi/wallet_transfer.go:238), history sync, and event dispatch.simulator(bool, line 76) — written bytest_connectivity, read during fee construction.Impact
These are data races under the Go memory model —
go run -race/go test -raceflags them immediately. Practical symptoms are stale or torn reads on 32-bit platforms and reordering surprises (e.g. transfer building reading adaemon_topoheightthat's inconsistent with the balance snapshot it pairs with). It also blocks ever running the race detector cleanly in CI, which hides new races (see the discussion on PR #26).Fix
Short term: switch to
sync/atomictypes (atomic.Int64,atomic.Bool) or guard with a small mutex.Long term: the file's own comment (line 74: "there should be no global variables, so multiple wallets can run at the same time") already points the way — move connection state into a client struct so multiple wallets/daemon connections can coexist in one process.