fix: keepalive data race

[vigodsky]

Hello,

Some data race fixes (lastResponse shared between ping handler and ticker goroutine)

[carlosmiei]

@vigodsky Thanks, we will review it soon

[Copilot]

Pull Request Overview

This PR fixes a data race condition in websocket keepalive functionality by making the lastResponse timestamp thread-safe using atomic operations. The lastResponse variable was being concurrently accessed by both the ping handler and ticker goroutine without synchronization.

• Replaced time.Time with int64 (Unix nanoseconds) for the lastResponse variable
• Added atomic operations for reading and writing the timestamp
• Applied the same fix consistently across all websocket implementations

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
v2/websocket.go	Fixed data race in keepAlive by using atomic operations for lastResponse timestamp
v2/portfolio/websocket.go	Applied identical data race fix to portfolio package keepAlive function
v2/options/websocket.go	Applied identical data race fix to options package keepAlive function
v2/futures/websocket.go	Applied identical data race fix to futures package keepAlive function
v2/delivery/websocket.go	Applied identical data race fix to delivery package keepAlive function

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

[nitpick] The explicit type declaration is unnecessary. Consider using short variable declaration: lastResponse := time.Now().UnixNano() for cleaner code.

Suggested change

	var lastResponse int64 = time.Now().UnixNano()
	lastResponse := time.Now().UnixNano()

[Copilot]

[nitpick] The explicit type declaration is unnecessary. Consider using short variable declaration: lastResponse := time.Now().UnixNano() for cleaner code.

Suggested change

	var lastResponse int64 = time.Now().UnixNano()
	lastResponse := time.Now().UnixNano()

[Copilot]

[nitpick] The explicit type declaration is unnecessary. Consider using short variable declaration: lastResponse := time.Now().UnixNano() for cleaner code.

Suggested change

	var lastResponse int64 = time.Now().UnixNano()
	lastResponse := time.Now().UnixNano()

[Copilot]

[nitpick] The explicit type declaration is unnecessary. Consider using short variable declaration: lastResponse := time.Now().UnixNano() for cleaner code.

Suggested change

	var lastResponse int64 = time.Now().UnixNano()
	lastResponse := time.Now().UnixNano()

[Copilot]

[nitpick] The explicit type declaration is unnecessary. Consider using short variable declaration: lastResponse := time.Now().UnixNano() for cleaner code.

Suggested change

	var lastResponse int64 = time.Now().UnixNano()
	lastResponse := time.Now().UnixNano()

[sc0Vu]

Should we keep lastResponse := ...?

[vigodsky]

fixed

[sc0Vu]

There is receiving loop that call ReceiveMessage() actively. I wonder if we can replace lastResponse with SetReadDeadline(). How do you think?

[vigodsky]

@sc0Vu

Looks good overall. One caveat: if the read loop gets blocked (i.e., we’re not inside ReadMessage/NextReader), we won’t observe the SetReadDeadline, so the timeout won’t trigger. Keeping the read pump non-blocking (or adding a thin watchdog to close on no progress) would cover that edge case.

I would suggest to keep ticker goroutine.

[sc0Vu]

LGTM

[sc0Vu]

@vigodsky We can keep ticker goroutine now.

For SetReadDeadline, I wonder if it's possible to do that in PongHandler, I saw gorilla and other project did that. They don't use ticker goroutine.