go-binance icon indicating copy to clipboard operation
go-binance copied to clipboard

Race condition in keepAlive() due to unsynchronized access to lastResponse

Open Never-Know-N opened this issue 7 months ago • 1 comments

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 avatar Apr 16 '25 20:04 Never-Know-N

@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.

sc0Vu avatar Apr 29 '25 11:04 sc0Vu

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

condrove10 avatar Jul 29 '25 10:07 condrove10