Skip to content

Commit

Permalink
Websocket: Fix race on DataHandler during shutdown
Browse files Browse the repository at this point in the history
Previously we would encounter situations where shutdown would race and
fail TestConnectionMessageErrors, demonstrating that consumers might not
be told why their connection is closing.

* Do not drain DataHandler on dataMonitor shutdown
Consumers should be allowed to process whatever messages were in flight
prior to a socket shutdown

* Ensure all DataHandler messages are sent to ToRoutine before shutdown
This avoids a race where we'd read a message from DataHandler, but then
process a shutdown before trying to relay it. The relay is non-blocking
anyway, so we can trust that we'd pick up the Shutdown during the next
loop.

* Send errors to DataHandler before starting a shutdown
This just reduces the chance of a race on shutdown processing
  • Loading branch information
gbjk committed Feb 26, 2024
1 parent cdbead3 commit 8228c88
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions exchanges/stream/websocket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,6 @@ func TestConnectionMessageErrors(t *testing.T) {
err = ws.Connect()
require.NoError(t, err, "Connect must not error")

ws.TrafficAlert <- struct{}{}

c := func(tb *assert.CollectT) {
select {
case v, ok := <-ws.ToRoutine:
Expand All @@ -345,6 +343,7 @@ func TestConnectionMessageErrors(t *testing.T) {
}
}

ws.TrafficAlert <- struct{}{}
ws.ReadMessageErrors <- errDastardlyReason
assert.EventuallyWithT(t, c, 2*time.Second, 10*time.Millisecond, "Should get an error down the routine")

Expand Down

0 comments on commit 8228c88

Please sign in to comment.