search

checksum0 / go-electrum

A pure Go ElectrumX JSON-RPC library.
MIT License
27 stars 26 forks source link

Panic in listenPush #10

Open pdyraga opened 1 year ago

pdyraga commented 1 year ago

When working on testnet and mainnet integration tests in https://github.com/keep-network/keep-core/pull/3599 we stumbled upon random panics when running tests:

=== FAIL: pkg/bitcoin/electrum TestGetLatestBlockHeight_Integration (unknown)
panic: assignment to entry in nil map

goroutine 511 [running]:
github.com/checksum0/go-electrum/electrum.(*Client).listenPush(0xc00036cc40, {0x120e9eb, 0x1c})
    /go/pkg/mod/github.com/keep-network/go-electrum@v0.0.0-20230524074410-befe891c2f8c/electrum/network.go:264 +0x74
github.com/checksum0/go-electrum/electrum.(*Client).SubscribeHeaders.func1()
    /go/pkg/mod/github.com/keep-network/go-electrum@v0.0.0-20230524074410-befe891c2f8c/electrum/subscribe.go:40 +0x36
created by github.com/checksum0/go-electrum/electrum.(*Client).SubscribeHeaders
    /go/pkg/mod/github.com/keep-network/go-electrum@v0.0.0-20230524074410-befe891c2f8c/electrum/subscribe.go:39 +0xfd

=== FAIL: pkg/bitcoin/electrum TestGetLatestBlockHeight_Integration/electrs-esplora_tcp_get (unknown)

=== FAIL: pkg/bitcoin/electrum TestGetLatestBlockHeight_Integration/electrumx_wss_get (unknown)
panic: assignment to entry in nil map

goroutine 511 [running]:
github.com/checksum0/go-electrum/electrum.(*Client).listenPush(0xc00036cc40, {0x[120](https://github.com/keep-network/keep-core/actions/runs/5215398427/jobs/9424824985?pr=3599#step:5:121)e9eb, 0x1c})
    /go/pkg/mod/github.com/keep-network/go-electrum@v0.0.0-20230524074410-befe891c2f8c/electrum/network.go:264 +0x74
github.com/checksum0/go-electrum/electrum.(*Client).SubscribeHeaders.func1()
    /go/pkg/mod/github.com/keep-network/go-electrum@v0.0.0-20230524074410-befe891c2f8c/electrum/subscribe.go:40 +0x36
created by github.com/checksum0/go-electrum/electrum.(*Client).SubscribeHeaders
    /go/pkg/mod/github.com/keep-network/go-electrum@v0.0.0-20230524074410-befe891c2f8c/electrum/subscribe.go:39 +0xfd

I think the problem lies in how the library handles closing the connection.

SubscribeHeaders creates a goroutine that writes to s.pushHandlers map in the listenPush function. If Shutdown() is called at the same time, unfortunate ordering of calls can lead to clearing s.pushHandlers = nil and then attempting to s.pushHandlers[method] = append(s.pushHandlers[method], c).

Separately from this issue, I think the goroutines created in SubscribeHeaders are leaking given there is no clear exit signal for them - the channel returned by listenPush is never closed and they are stuck on reading.