do not block the read routine on a late reject
The connect channel was unbuffered. Once DialWithDialer has returned on its synchronization timeout nothing reads that channel again, so a Reject arriving afterwards blocked handleReject, and with it readRoutine, forever. That leaks the goroutine along with the client, its user and channel maps and its read buffer, and because readRoutine never reaches its exit path the ping and UDP routines are never signalled to stop either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
fe322ce067
commit
53894b33ae
@@ -181,7 +181,12 @@ func DialWithDialer(dialer *net.Dialer, config *Config, tlsConfig *tls.Config) (
|
|||||||
|
|
||||||
state: uint32(StateConnected),
|
state: uint32(StateConnected),
|
||||||
|
|
||||||
connect: make(chan *RejectError),
|
// Buffered: once DialWithDialer returns on its synchronization
|
||||||
|
// timeout nothing reads this channel again, and an unbuffered send
|
||||||
|
// from handleReject would block readRoutine forever, leaking the
|
||||||
|
// goroutine along with the client, its user and channel maps and its
|
||||||
|
// multi-megabyte read buffer.
|
||||||
|
connect: make(chan *RejectError, 1),
|
||||||
end: make(chan struct{}),
|
end: make(chan struct{}),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,56 @@
|
|||||||
|
package gumble
|
||||||
|
|
||||||
|
import (
|
||||||
|
"io"
|
||||||
|
"net"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"git.stormux.org/storm/barnard/gumble/gumble/MumbleProto"
|
||||||
|
"google.golang.org/protobuf/proto"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Regression: the connect channel was unbuffered, so a Reject arriving after
|
||||||
|
// DialWithDialer had already returned on its synchronization timeout blocked
|
||||||
|
// readRoutine forever, leaking that goroutine and the whole client with it.
|
||||||
|
func TestHandleRejectDoesNotBlockWithoutAReceiver(t *testing.T) {
|
||||||
|
c := &Client{
|
||||||
|
Config: NewConfig(),
|
||||||
|
Users: make(Users),
|
||||||
|
connect: make(chan *RejectError, 1),
|
||||||
|
state: uint32(StateConnected),
|
||||||
|
}
|
||||||
|
c.Conn = NewConn(nopConn{})
|
||||||
|
|
||||||
|
reason := "server is full"
|
||||||
|
data, _ := proto.Marshal(&MumbleProto.Reject{Reason: &reason})
|
||||||
|
|
||||||
|
done := make(chan struct{})
|
||||||
|
go func() {
|
||||||
|
_ = c.handleReject(data)
|
||||||
|
close(done)
|
||||||
|
}()
|
||||||
|
|
||||||
|
select {
|
||||||
|
case <-done:
|
||||||
|
case <-timeoutChan():
|
||||||
|
t.Fatal("handleReject blocked with no reader on the connect channel")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// nopConn is a net.Conn that discards everything, so handleReject's Close call
|
||||||
|
// has something to act on.
|
||||||
|
type nopConn struct{}
|
||||||
|
|
||||||
|
func (nopConn) Read(b []byte) (int, error) { return 0, io.EOF }
|
||||||
|
func (nopConn) Write(b []byte) (int, error) { return len(b), nil }
|
||||||
|
func (nopConn) Close() error { return nil }
|
||||||
|
func (nopConn) LocalAddr() net.Addr { return nil }
|
||||||
|
func (nopConn) RemoteAddr() net.Addr { return nil }
|
||||||
|
func (nopConn) SetDeadline(t time.Time) error { return nil }
|
||||||
|
func (nopConn) SetReadDeadline(t time.Time) error { return nil }
|
||||||
|
func (nopConn) SetWriteDeadline(t time.Time) error { return nil }
|
||||||
|
|
||||||
|
func timeoutChan() <-chan time.Time {
|
||||||
|
return time.After(10 * time.Second)
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user