From c4d2db417749c721c6ec5b630e517a11a3ecb457 Mon Sep 17 00:00:00 2001 From: Abdellatif Anaflous <62770500+hktitof@users.noreply.github.com> Date: Sat, 10 Oct 2026 11:19:59 +0100 Subject: [PATCH] fix(websocket): release a pending Accept when the listener is closed (#5552) the websocket listener kept its accept channel open on Close, so an Accept call that was already waiting blocked forever every other listener in pkg/util/net closes the channel and returns an error, but this one never did, so the HandleListener goroutine stayed stuck after the server closed i close the channel on Close, guard the hand off send so a late connection does not panic, and return ErrWebsocketListenerClosed which was declared but never reached Signed-off-by: Abdel <62770500+hktitof@users.noreply.github.com> --- pkg/util/net/websocket.go | 24 ++++++++++- pkg/util/net/websocket_test.go | 73 ++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 pkg/util/net/websocket_test.go diff --git a/pkg/util/net/websocket.go b/pkg/util/net/websocket.go index 57641b31..0124fc83 100644 --- a/pkg/util/net/websocket.go +++ b/pkg/util/net/websocket.go @@ -4,8 +4,10 @@ import ( "errors" "net" "net/http" + "sync" "time" + gerr "github.com/fatedier/golib/errors" "golang.org/x/net/websocket" ) @@ -20,6 +22,9 @@ type WebsocketListener struct { acceptCh chan net.Conn server *http.Server + + mu sync.Mutex + closed bool } // NewWebsocketListener to handle websocket connections @@ -41,7 +46,15 @@ func NewWebsocketListener(ln net.Listener) (wl *WebsocketListener) { conn := WrapCloseNotifyConn(c, func(_ error) { close(notifyCh) }) - wl.acceptCh <- conn + // The listener may be closed while this connection is being handed + // over, so the send has to tolerate a closed channel. A nil error + // means the connection was accepted and is owned by the caller. + if err := gerr.PanicToError(func() { + wl.acceptCh <- conn + }); err != nil { + conn.Close() + return + } <-notifyCh })) @@ -66,6 +79,15 @@ func (p *WebsocketListener) Accept() (net.Conn, error) { } func (p *WebsocketListener) Close() error { + p.mu.Lock() + if !p.closed { + p.closed = true + // Closing acceptCh releases a pending Accept, matching the other + // listeners in this package. Otherwise it blocks forever and the + // caller never learns that the listener is closed. + close(p.acceptCh) + } + p.mu.Unlock() return p.server.Close() } diff --git a/pkg/util/net/websocket_test.go b/pkg/util/net/websocket_test.go new file mode 100644 index 00000000..f0f6b9d9 --- /dev/null +++ b/pkg/util/net/websocket_test.go @@ -0,0 +1,73 @@ +// Copyright 2026 The frp Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package net + +import ( + "errors" + "net" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +type closeStubListener struct { + closed chan struct{} +} + +func (l *closeStubListener) Accept() (net.Conn, error) { + <-l.closed + return nil, net.ErrClosed +} + +func (l *closeStubListener) Close() error { + select { + case <-l.closed: + default: + close(l.closed) + } + return nil +} + +func (l *closeStubListener) Addr() net.Addr { + return &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1)} +} + +func TestWebsocketListenerAcceptUnblocksOnClose(t *testing.T) { + wl := NewWebsocketListener(&closeStubListener{closed: make(chan struct{})}) + + errCh := make(chan error, 1) + go func() { + _, err := wl.Accept() + errCh <- err + }() + + select { + case err := <-errCh: + t.Fatalf("Accept returned before the listener was closed: %v", err) + case <-time.After(100 * time.Millisecond): + } + + require.NoError(t, wl.Close()) + // Closing twice must stay safe and keep reporting the closed error. + require.NoError(t, wl.Close()) + + select { + case err := <-errCh: + require.True(t, errors.Is(err, ErrWebsocketListenerClosed), "got %v", err) + case <-time.After(time.Second): + t.Fatal("Accept did not return after the listener was closed") + } +}