Skip to content

Commit 84c5e74

Browse files
authored
Fix write lock deadline handling (#7)
## Issue The existing code in go-algorand is using two go-routines for reading/writing and a separate go-routine for opening/closing the connections. When a client connection becomes non-responsive ( i.e. the writer is not returning ), the current implementation tries to close the connection. However, per WebSocket specification, the client is supposed to send a Close message first. The close message is being sent using the `WriteControl` method, which is supposed to be able to run concurrently with `WriteMessage`. However, both reaches `Conn.write` where it attempt to acquire a writing lock. Since the `WriteMessage -> Conn.write` is stuck ( but the `WriteControl` doesn't really know it's stuck indefinitely ), the `WriteControl -> Conn.write` get blocks indefinitely as well. Unlike the `WriteMessage`, when calling `WriteControl`, we provide a deadline : a timestamp after which we don't care if the message was actually sent. In this PR, I have added handling that would limit the time the `Conn.write` would wait to that deadline. From spec perspective, it would be a violation of the WebSocket protocol; however, given that the other party is no longer responsive, a successful writing of a Close message is not possible, it would probably be just fine.
1 parent 19af735 commit 84c5e74

File tree

1 file changed

+34
-1
lines changed

1 file changed

+34
-1
lines changed

conn.go

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ var ErrCloseSent = errors.New("websocket: close sent")
9090
// read limit set for the connection.
9191
var ErrReadLimit = errors.New("websocket: read limit exceeded")
9292

93+
// ErrWriteDeadlineExceeded is returned when a writing to a connection has exceeded
94+
// the specified deadline
95+
var ErrWriteDeadlineExceeded = errors.New("websocket: writing deadline exceeded")
96+
9397
// netError satisfies the net Error interface.
9498
type netError struct {
9599
msg string
@@ -428,7 +432,36 @@ func (c *Conn) read(n int) ([]byte, error) {
428432
}
429433

430434
func (c *Conn) write(frameType int, deadline time.Time, buf0, buf1 []byte) error {
431-
<-c.mu
435+
// if user has provided no deadline
436+
if deadline.IsZero() {
437+
// just wait for the mutex.
438+
<-c.mu
439+
} else {
440+
// see how far the deadline is from now
441+
sleepTime := deadline.Sub(time.Now())
442+
443+
// is the deadline pointing to the future, or the past ?
444+
if sleepTime < 0 {
445+
// operation timed out already.
446+
return ErrWriteDeadlineExceeded
447+
}
448+
449+
select {
450+
case <-c.mu:
451+
// good, we've got the writing lock.
452+
453+
// the default case would be taken if the above lock could not be aquired right away.
454+
default:
455+
// writing lock is currently taken.
456+
select {
457+
case <-c.mu:
458+
// great ! we got the writing lock.
459+
case <-time.After(sleepTime):
460+
// we were not able to aquire the writing lock.
461+
return ErrWriteDeadlineExceeded
462+
}
463+
}
464+
}
432465
defer func() { c.mu <- true }()
433466

434467
c.writeErrMu.Lock()

0 commit comments

Comments
 (0)