go-binance
go-binance copied to clipboard
Race condition in keepAlive() due to unsynchronized access to lastResponse
https://github.com/ccxt/go-binance/blob/8d83c8396b8a799c16df475cb1afc10cd044ce37/v2/futures/websocket.go#L86-L117
Hello team 👋
I encountered a data race while running go run -race using the go-binance/v2 client in a production-like environment.
The race happens inside the keepAlive() function in websocket.go, where the lastResponse variable is: • Written inside the SetPingHandler goroutine • Read inside the ticker goroutine
WARNING: DATA RACE
Read at ... by goroutine A:
go-binance/v2/futures.keepAlive.func2()
↳ websocket.go:111
Previous write at ... by goroutine B:
go-binance/v2/futures.keepAlive.func1()
↳ websocket.go:102
gorilla/websocket.(*Conn).advanceFrame()
↳ conn.go:954
gorilla/websocket.(*Conn).NextReader()
gorilla/websocket.(*Conn).ReadMessage()
Goroutine A created at:
go-binance/v2/futures.keepAlive()
↳ websocket.go:107
Goroutine B created at:
go-binance/v2/futures.WsUserDataServe()
↳ websocket_service.go:1198
While it doesn’t crash, this race condition raises concerns during development—especially when running alongside other critical systems—since it triggers race warnings that can undermine confidence in stability. Thanks for maintaining such a useful library!
@Never-Know-N Thanks for reporting this. You're right about the race condition. The lastResponse doesn't look good there. I'll see how to fix this.
Are there any updates on this issue? Has the draft been tested? Could you create a staging stack and test your application importing the package with the commit hash in the draft? @Never-Know-N
Alternatively could we consider the integration of a local context or mutex to better manage the websocket connection closure in case the ping/pong fails? @sc0Vu