Skip to content

Commit 85843c5

Browse files
committed
fix: WebSocketConnection: Timeout on Read shuts down writing connection
webSocketConnection Read and Write had separate timeouts which, when expired, mutually closed the websocket. In case only one party of the connection was sending, the websocket was closed after the timeout despite the fact that the connection was alive. This comes from misunderstanding the impact of the Read/Write context parameter. See coder/websocket#242
1 parent 0b9f47b commit 85843c5

File tree

1 file changed

+64
-16
lines changed

1 file changed

+64
-16
lines changed

websocketconnection.go

+64-16
Original file line numberDiff line numberDiff line change
@@ -4,44 +4,41 @@ import (
44
"bytes"
55
"context"
66
"fmt"
7-
87
"github.com/teivah/onecontext"
98
"nhooyr.io/websocket"
9+
"time"
1010
)
1111

1212
type webSocketConnection struct {
1313
ConnectionBase
1414
conn *websocket.Conn
1515
transferMode TransferMode
16+
watchDogChan chan dogFood
1617
}
1718

1819
func newWebSocketConnection(parentContext context.Context, requestContext context.Context, connectionID string, conn *websocket.Conn) *webSocketConnection {
1920
ctx, _ := onecontext.Merge(parentContext, requestContext)
2021
w := &webSocketConnection{
21-
conn: conn,
22+
conn: conn,
23+
watchDogChan: make(chan dogFood, 1),
2224
ConnectionBase: ConnectionBase{
2325
ctx: ctx,
2426
connectionID: connectionID,
2527
},
2628
}
29+
go w.watchDog(ctx)
2730
return w
2831
}
2932

3033
func (w *webSocketConnection) Write(p []byte) (n int, err error) {
3134
if err := w.Context().Err(); err != nil {
3235
return 0, fmt.Errorf("webSocketConnection canceled: %w", w.ctx.Err())
3336
}
34-
ctx := w.ctx
35-
if w.timeout > 0 {
36-
var cancel context.CancelFunc
37-
ctx, cancel = context.WithTimeout(w.ctx, w.Timeout())
38-
defer cancel() // has no effect because timeoutCtx is either done or not used anymore after websocket returns. But it keeps lint quiet
39-
}
4037
messageType := websocket.MessageText
4138
if w.transferMode == BinaryTransferMode {
4239
messageType = websocket.MessageBinary
4340
}
44-
err = w.conn.Write(ctx, messageType, p)
41+
err = w.conn.Write(w.resetWatchDog(), messageType, p)
4542
if err != nil {
4643
return 0, err
4744
}
@@ -52,19 +49,70 @@ func (w *webSocketConnection) Read(p []byte) (n int, err error) {
5249
if err := w.Context().Err(); err != nil {
5350
return 0, fmt.Errorf("webSocketConnection canceled: %w", w.ctx.Err())
5451
}
55-
ctx := w.ctx
56-
if w.timeout > 0 {
57-
var cancel context.CancelFunc
58-
ctx, cancel = context.WithTimeout(w.ctx, w.Timeout())
59-
defer cancel() // has no effect because timeoutCtx is either done or not used anymore after websocket returns. But it keeps lint quiet
60-
}
61-
_, data, err := w.conn.Read(ctx)
52+
_, data, err := w.conn.Read(w.resetWatchDog())
6253
if err != nil {
6354
return 0, err
6455
}
6556
return bytes.NewReader(data).Read(p)
6657
}
6758

59+
// resetWatchDog resets the common watchDog for Read and Write.
60+
// the watchDog will stop waiting for the last set timeout and wait for the new timeout.
61+
func (w *webSocketConnection) resetWatchDog() context.Context {
62+
ctx := w.ctx
63+
food := dogFood{timeout: w.timeout}
64+
if w.timeout > 0 {
65+
ctx, food.bark = context.WithCancel(w.ctx)
66+
}
67+
w.watchDogChan <- food
68+
return ctx
69+
}
70+
71+
// dogFood is used to reset the watchDog
72+
type dogFood struct {
73+
// After this, the dog will bark
74+
timeout time.Duration
75+
bark context.CancelFunc
76+
}
77+
78+
// watchDog is the common watchDog for Read and Write. It stops the connection (aka closes the Websocket)
79+
// when the last timeout has elapsed. If resetWatchDog is called before the last timeout has elapsed,
80+
// the watchDog will restart waiting for the new timeout. If timeout is set to 0, it will not wait at all.
81+
func (w *webSocketConnection) watchDog(ctx context.Context) {
82+
var timer *time.Timer
83+
var cancelTimeoutChan chan struct{}
84+
for {
85+
select {
86+
case <-ctx.Done():
87+
return
88+
case food := <-w.watchDogChan:
89+
if timer != nil {
90+
if !timer.Stop() {
91+
go func() {
92+
<-timer.C
93+
}()
94+
}
95+
go func() {
96+
cancelTimeoutChan <- struct{}{}
97+
}()
98+
}
99+
if food.timeout != 0 {
100+
timer = time.NewTimer(food.timeout)
101+
cancelTimeoutChan = make(chan struct{}, 1)
102+
go func() {
103+
select {
104+
case <-cancelTimeoutChan:
105+
case <-timer.C:
106+
food.bark()
107+
}
108+
}()
109+
} else {
110+
timer = nil
111+
}
112+
}
113+
}
114+
}
115+
68116
func (w *webSocketConnection) TransferMode() TransferMode {
69117
return w.transferMode
70118
}

0 commit comments

Comments
 (0)