Summary
The package-level watchdog timer is Reset concurrently from two goroutines with no synchronization and no drain discipline:
Notify_broadcaster (runs on the jrpc2 notification callback goroutine): timer.Reset(timeout) —
|
timer.Reset(timeout) // connection is alive |
Keep_Connectivity loop: timer.Reset(timeout) right after <-timer.C —
|
case <-timer.C: // we disconnected and did not connect, this timer fires every 5 secs, |
|
|
|
timer.Reset(timeout) |
|
if !Connected { |
Impact
time.Timer.Reset's documented contract requires the timer to be stopped/drained before Reset when the channel may have fired; calling Reset from a second goroutine while another is selecting on timer.C can deliver a stale expiry (a ping/reconnect cycle triggered even though a notification just proved the connection alive) or, less commonly, delay the watchdog by a full period. It's low-severity — the watchdog self-corrects on the next cycle — but it makes the liveness window nondeterministic, which matters more once the timeout is shortened (PR #28) and the per-block connectivity call is removed (PR #29), leaving this timer as the primary liveness signal.
Fix
Drop the shared timer entirely: record lastSeen (an atomic.Int64 unix-nano stamp) in Notify_broadcaster, and have Keep_Connectivity tick on its own private time.Ticker, pinging only when time.Since(lastSeen) > timeout. Single writer per primitive, no Reset semantics to get wrong.
Summary
The package-level watchdog timer is
Resetconcurrently from two goroutines with no synchronization and no drain discipline:Notify_broadcaster(runs on the jrpc2 notification callback goroutine):timer.Reset(timeout)—derohe/walletapi/daemon_communication.go
Line 102 in 6f9bd2c
Keep_Connectivityloop:timer.Reset(timeout)right after<-timer.C—derohe/walletapi/daemon_connectivity_loop.go
Lines 42 to 45 in 6f9bd2c
Impact
time.Timer.Reset's documented contract requires the timer to be stopped/drained before Reset when the channel may have fired; callingResetfrom a second goroutine while another is selecting ontimer.Ccan deliver a stale expiry (a ping/reconnect cycle triggered even though a notification just proved the connection alive) or, less commonly, delay the watchdog by a full period. It's low-severity — the watchdog self-corrects on the next cycle — but it makes the liveness window nondeterministic, which matters more once the timeout is shortened (PR #28) and the per-block connectivity call is removed (PR #29), leaving this timer as the primary liveness signal.Fix
Drop the shared timer entirely: record
lastSeen(anatomic.Int64unix-nano stamp) inNotify_broadcaster, and haveKeep_Connectivitytick on its own privatetime.Ticker, pinging only whentime.Since(lastSeen) > timeout. Single writer per primitive, no Reset semantics to get wrong.