keep-network / go-electrum

A pure Go ElectrumX JSON-RPC library.
MIT License
0 stars 10 forks source link

Panic in listenPush #4

Open pdyraga opened 1 year ago

pdyraga commented 1 year ago

Originally posted in https://github.com/checksum0/go-electrum/issues/10.

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.

michalinacienciala commented 1 year ago

Some new cases of panic failures, after https://github.com/keep-network/keep-core/pull/3656 got merged to main:

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

goroutine 545 [running]:
github.com/checksum0/go-electrum/electrum.(*Client).listenPush(0xc0002aecb0, {0x120fbc8, 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/fulcrum_tcp_get (unknown)
panic: assignment to entry in nil map

goroutine 757 [running]:
github.com/checksum0/go-electrum/electrum.(*Client).listenPush(0xc000310c40, {0x121655d, 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 (unknown)
    electrum_integration_test.go:437: failed to initialize electrum client: [retry timeout [1m0s] exceeded; most recent error: [websocket: bad handshake]]
panic: assignment to entry in nil map

goroutine 757 [running]:
github.com/checksum0/go-electrum/electrum.(*Client).listenPush(0xc000310c40, {0x121655d, 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/embedded/mainnet/wss://electrumx-server.tbtc.network:8443_get (unknown)
panic: assignment to entry in nil map

goroutine 566 [running]:
github.com/checksum0/go-electrum/electrum.(*Client).listenPush(0xc0024d2930, {0x[120](https://github.com/keep-network/keep-core/actions/runs/5459180187/jobs/9935141566#step:5:121)fbc8, 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
michalinacienciala commented 1 year ago

Another case: https://github.com/keep-network/keep-core/actions/runs/5663040756/job/15344593515.

=== FAIL: pkg/bitcoin/electrum TestGetLatestBlockHeight_Integration/embedded/mainnet/wss://electrumx.prod-utility-eks-us-west-2.staked.cloud:443_get (unknown)
panic: assignment to entry in nil map

goroutine 595 [running]:
github.com/checksum0/go-electrum/electrum.(*Client).listenPush(0xc0000aed20, {0x[121](https://github.com/keep-network/keep-core/actions/runs/5663040756/job/15344593515#step:5:122)6560, 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