Gareth Kirwan ce3d29f5d5 Okx: Fix panic during shutdown due to race (#1240) ...

* Okx: Fix panic during shutdown due to race

Error:
```
panic: runtime error: slice bounds out of range [:13340] with capacity 8192
bufio.(*Reader).Read(0x14000038540, {0x1400059cffc?, 0x4?, 0x0?})
        /usr/local/go/src/bufio/bufio.go:250 +0x33c
github.com/gorilla/websocket.(*messageReader).Read(0x1400098bb78, {0x1400059cffc, 0x4, 0x4})
        /Users/gbjk/go/pkg/mod/github.com/gorilla/websocket@v1.5.0/conn.go:1050 +0x208
io.ReadAll({0x12f36c618, 0x1400098bb78})
        /usr/local/go/src/io/io.go:701 +0xe4
io/ioutil.ReadAll(...)
        /usr/local/go/src/io/ioutil/ioutil.go:27
github.com/gorilla/websocket.(*Conn).ReadMessage(0x3?)
        /Users/gbjk/go/pkg/mod/github.com/gorilla/websocket@v1.5.0/conn.go:1097 +0x54
github.com/thrasher-corp/gocryptotrader/exchanges/stream.(*WebsocketConnection).ReadMessage(0x140006622d0)
        /Users/gbjk/go/pkg/mod/github.com/gbjk/gocryptotrader@v0.0.0-20230619070715-ae6f283f6be6/exchanges/stream/websocket_connection.go:217 +0x30
github.com/thrasher-corp/gocryptotrader/exchanges/okx.(*Okx).wsFunnelConnectionData(0x140019321e0?, {0x106909450, 0x140006622d0})
        /Users/gbjk/go/pkg/mod/github.com/gbjk/gocryptotrader@v0.0.0-20230619070715-ae6f283f6be6/exchanges/okx/okx_websocket.go:346 +0x94
created by github.com/thrasher-corp/gocryptotrader/exchanges/okx.(*Okx).WsConnect
        /Users/gbjk/go/pkg/mod/github.com/gbjk/gocryptotrader@v0.0.0-20230619070715-ae6f283f6be6/exchanges/okx/okx_websocket.go:234 +0x134
exit status 2
```
This happens when there's a race in calls to bufio because it over-reads. See [this comment](https://github.com/golang/go/issues/42289#issuecomment-723393783)
Detected using go -race:
```
WARNING: DATA RACE
Read at 0x00c000818bc0 by goroutine 2156:
  github.com/gorilla/websocket.(*Conn).NextReader()
      /Users/gbjk/go/pkg/mod/github.com/gorilla/websocket@v1.5.0/conn.go:1000 +0x38
  github.com/gorilla/websocket.(*Conn).ReadMessage()
      /Users/gbjk/go/pkg/mod/github.com/gorilla/websocket@v1.5.0/conn.go:1093 +0x28
  github.com/thrasher-corp/gocryptotrader/exchanges/stream.(*WebsocketConnection).ReadMessage()
      /Users/gbjk/go/pkg/mod/github.com/gbjk/gocryptotrader@v0.0.0-20230619070715-ae6f283f6be6/exchanges/stream/websocket_connection.go:217 +0x44
  github.com/thrasher-corp/gocryptotrader/exchanges/okx.(*Okx).wsFunnelConnectionData()
```

Because we started a new wsFunnelConnectionData for each re-connect.

This bug might apply to other exchanges.

* Okx: Fix websocket waitgroup going negative

Move the waitgroup additions to the actual places that use them

* Okx: Add nolint for revive

* Okx: Move wg Adds to outside goros

There is a risk of a race condition if we let the goros Add themselves.

* Okx: Simplify websocket reading

This fixes the issue that the WsRead and Multiplexer were intrinsically
linked to the websocket, even though they need to survive both
disconnects and Disable/Enables.

Messages are now handled in a goro, which means they might not be
sequential, but there's a very high chance that messages of the same
codepath will be handled sequentially. So orderbook, ticked and order
messages should be sequential

* Okx: Switch to blocking processing of ws msgs

* Okx: Remove nolint from Setup

Actioning a review comment: @gloriousCode prefers to avoid having to nolint this in favour of
a func call return.

* Okx: Remove redundant Wg use inside WsReadData

* Okx: Fix WsMultiplexer Re-Run() shutdown

2023-07-17 13:21:28 +10:00

48 KiB

Raw Blame History

View Raw