WriteMessage does not respect deadline acquring mutex [bug]

[rittneje]

Unlike WriteControl, WriteMessage does not attempt to honor the deadline while acquiring the mutex.

WriteMessage

websocket/conn.go

Lines 378 to 380 in b65e629

	func (c *Conn) write(frameType int, deadline time.Time, buf0, buf1 []byte) error {
	<-c.mu
	defer func() { c.mu <- struct{}{} }()

WriteControl

websocket/conn.go

Lines 432 to 447 in b65e629

	d := 1000 * time.Hour
	if !deadline.IsZero() {
	d = deadline.Sub(time.Now())
	if d < 0 {
	return errWriteTimeout
	}
	}
	timer := time.NewTimer(d)
	select {
	case <-c.mu:
	timer.Stop()
	case <-timer.C:
	return errWriteTimeout
	}
	defer func() { c.mu <- struct{}{} }()

It's not clear whether this can actually cause an issue, but it is certainly suspicious.

[ghost]

WriteMessage does not compete with other calls to WriteMessage because only one concurrent writer is supported.

The problem is that WriteMessage can wait longer than specified deadline when WriteMessage waits on a call to WriteControl. This is probably not a big problem in practice, but the problem should be fixed.

Fix by creating a function to acquire the mutex with deadline and call that function from both code paths. The function should avoid creating the ticker when there's no contention.

// aquireWriteMutex returns true if the write mutex was aquired before 
// the specified deadline.
func (c *Conn) aquireWriteMutex(deadline time.Time) bool {
        if deadline.IsZero() {
            <-c.mu
            return true
        }

	d = deadline.Sub(time.Now())
	if d < 0 {
	    return false
	}

	select {
	case <-c.mu:
	     return true
	default:
	    timer := time.NewTimer(d)
	    select {
	    case <-c.mu:
	        timer.Stop()
	        return true
	    case <-timer.C:
	        return false
	    }
    }

[wuchihsu]

WriteMessage does not compete with other calls to WriteMessage because only one concurrent writer is supported.

The problem is that WriteMessage can wait longer than specified deadline when WriteMessage waits on a call to WriteControl. This is probably not a big problem in practice, but the problem should be fixed.

Fix by creating a function to acquire the mutex with deadline and call that function from both code paths. The function should avoid creating the ticker when there's no contention.

// aquireWriteMutex returns true if the write mutex was aquired before 
// the specified deadline.
func (c *Conn) aquireWriteMutex(deadline time.Time) bool {
        if deadline.IsZero() {
            <-c.mu
            return true
        }

	d = deadline.Sub(time.Now())
	if d < 0 {
	    return false
	}

	select {
	case <-c.mu:
	     return true
	default:
	    timer := time.NewTimer(d)
	    select {
	    case <-c.mu:
	        timer.Stop()
	        return true
	    case <-timer.C:
	        return false
	    }
    }

Hi, I'm trying to understand this. Do this mean to use aquireWriteMutex in both write and WriteControl?