search

HuobiRDCenter / huobi_Golang

Go SDK for Huobi Spot API
https://www.htx.com/zh-cn/opend/newApiPages/
Apache License 2.0
176 stars 86 forks source link

WebSocketV1ClientBase.lastReceivedTime field data race #20

Closed filinvadim closed 3 years ago

filinvadim commented 4 years ago 
WARNING: DATA RACE
Write at 0x00c000158128 by goroutine 56:
  .../pkg/client/websocketclientbase.(*WebSocketV1ClientBase).readLoop()
     .../pkg/client/websocketclientbase/websocketv1clientbase.go:212 +0x23c

Previous write at 0x00c000158128 by goroutine 16:
 .../pkg/client/websocketclientbase.(*WebSocketV1ClientBase).startTicker()
     .../pkg/client/websocketclientbase/websocketv1clientbase.go:137 +0xbc
filinvadim commented 4 years ago

Just:

p.timeMutex.Lock()
p.lastReceivedTime = time.Now()
p.timeMutex.Unlock()
filinvadim commented 4 years ago

Same for WebSocketV2ClientBase

eynzhang commented 3 years ago

@filinvadim thanks for the feedback, usually this won’t happen, however it has possibility in theory as lastReceivedTime is written in two routines. I don’t want to introduce mutex, so just move the lastReceivedTime initialization to earlier places. You can check the latest master branch.

filinvadim commented 3 years ago

@eynzhang so data race is still in place. You need a mutex, sorry.

eynzhang commented 3 years ago

@filinvadim could you share how you reproduce this issue?

filinvadim commented 3 years ago

Sorry no, @eynzhang . It's in your code. Just go run --race ... You have access to the variable lastReceivedTime from 2 threads - tickerLoop and readLoop. That's data race. Reopen please to warn other developers.