Summary
Fix deadlock when an ElectrumX connection pinger calls (*clientConn).Close(), discovered by @ClaytonNorthey92.
Close previously waited on the pinger to exit before setting closeCh = nil, so that closeCh != nil could be checked to prevent panics by closing the channel more than once. This resulted in a deadlock when Close was called inside the pinger goroutine.
Instead of using the closeCh channel, use a cancelable context stored in the clientConn struct. This isn't the best approach, but is simple and doesn't have double close issues. This also allows us to use this context when pinging ElectrumX, allowing the ping to be canceled when the connection is closed.
Additionally, this stops the ticker used in the pinger goroutine. Failing to stop time.Tickers prior to Go 1.23 results in them never being collected during GC. https://go.dev/doc/go1.23#timer-changes
Changes
Fix deadlock when an ElectrumX connection pinger calls (*clientConn).Close().
Fix unstopped ticker in ElectrumX connection pinger goroutine.
Summary Fix deadlock when an ElectrumX connection pinger calls
(*clientConn).Close(), discovered by @ClaytonNorthey92.Close previously waited on the pinger to exit before setting
closeCh = nil, so thatcloseCh != nilcould be checked to prevent panics by closing the channel more than once. This resulted in a deadlock when Close was called inside thepingergoroutine.Instead of using the
closeChchannel, use a cancelable context stored in theclientConnstruct. This isn't the best approach, but is simple and doesn't have double close issues. This also allows us to use this context when pinging ElectrumX, allowing the ping to be canceled when the connection is closed.Additionally, this stops the ticker used in the
pingergoroutine. Failing to stoptime.Tickers prior to Go 1.23 results in them never being collected during GC. https://go.dev/doc/go1.23#timer-changesChanges
(*clientConn).Close().