Skip to content

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

@Never-Know-N

Description

@Never-Know-N

func keepAlive(c *websocket.Conn, timeout time.Duration) {
ticker := time.NewTicker(timeout)
lastResponse := time.Now()
c.SetPingHandler(func(pingData string) error {
// Respond with Pong using the server's PING payload
err := c.WriteControl(
websocket.PongMessage,
[]byte(pingData),
time.Now().Add(WebsocketPongTimeout), // Short deadline to ensure timely response
)
if err != nil {
return err
}
lastResponse = time.Now()
return nil
})
go func() {
defer ticker.Stop()
for {
<-ticker.C
if time.Since(lastResponse) > timeout {
c.Close()
return
}
}
}()
}

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!

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions