Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove deadlock in multiPacketListener #211

Merged
merged 12 commits into from
Sep 18, 2024
83 changes: 43 additions & 40 deletions service/listeners.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,9 @@ type readRequest struct {

type virtualPacketConn struct {
net.PacketConn
mu sync.Mutex // Mutex to protect access to the channels
readCh chan readRequest
onCloseFunc OnCloseFunc
mu sync.Mutex // Mutex to protect access to the channels
readCh chan readRequest
closeCh chan struct{}
}

func (pc *virtualPacketConn) ReadFrom(p []byte) (n int, addr net.Addr, err error) {
sbruens marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -162,13 +162,9 @@ func (pc *virtualPacketConn) Close() error {
if pc.readCh == nil {
return nil
}
pc.readCh = nil

if pc.onCloseFunc != nil {
onCloseFunc := pc.onCloseFunc
pc.onCloseFunc = nil
return onCloseFunc()
}
close(pc.closeCh)
pc.readCh = nil
return nil
}

Expand Down Expand Up @@ -286,44 +282,51 @@ func (m *multiPacketListener) Acquire() (net.PacketConn, error) {
m.pc = pc
m.readCh = make(chan readRequest)
m.doneCh = make(chan struct{})
go func() {
for {
select {
case req := <-m.readCh:
n, addr, err := pc.ReadFrom(req.buffer)
req.respCh <- struct {
n int
addr net.Addr
err error
}{n, addr, err}
case <-m.doneCh:
return
}
}
}()
}

m.count++
return &virtualPacketConn{
vpc := &virtualPacketConn{
PacketConn: m.pc,
readCh: m.readCh,
onCloseFunc: func() error {
m.mu.Lock()
defer m.mu.Unlock()
m.count--
if m.count == 0 {
close(m.doneCh)
m.pc.Close()
m.pc = nil
if m.onCloseFunc != nil {
onCloseFunc := m.onCloseFunc
m.onCloseFunc = nil
return onCloseFunc()
closeCh: make(chan struct{}),
}

go func() {
sbruens marked this conversation as resolved.
Show resolved Hide resolved
for {
select {
case req, ok := <-m.readCh:
if !ok {
// The read channel is closed.
return
}
n, addr, err := m.pc.ReadFrom(req.buffer)
req.respCh <- struct {
n int
addr net.Addr
err error
}{n, addr, err}
case <-vpc.closeCh:
m.mu.Lock()
defer m.mu.Unlock()
m.count--
if m.count == 0 {
close(m.doneCh)
m.pc.Close()
m.pc = nil
if m.onCloseFunc != nil {
onCloseFunc := m.onCloseFunc
m.onCloseFunc = nil
onCloseFunc()
}
}
return
case <-m.doneCh:
return
}
return nil
},
}, nil
}
}()

return vpc, nil
}

// ListenerManager holds the state of shared listeners.
Expand Down
2 changes: 1 addition & 1 deletion service/listeners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func TestListenerManagerPacketListenerEarlyClose(t *testing.T) {
_, writeErr := pc.WriteTo(nil, &net.UDPAddr{})

require.ErrorIs(t, readErr, net.ErrClosed)
require.ErrorIs(t, writeErr, net.ErrClosed)
require.ErrorContains(t, writeErr, "sendto: invalid argument")
}

func TestListenerManagerPacketListenerNotClosedIfStillInUse(t *testing.T) {
Expand Down
Loading