mirror of
https://github.com/fatedier/frp.git
synced 2026-10-15 04:35:55 +08:00
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>
This commit is contained in:
@@ -4,8 +4,10 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"net"
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
gerr "github.com/fatedier/golib/errors"
|
||||||
"golang.org/x/net/websocket"
|
"golang.org/x/net/websocket"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -20,6 +22,9 @@ type WebsocketListener struct {
|
|||||||
acceptCh chan net.Conn
|
acceptCh chan net.Conn
|
||||||
|
|
||||||
server *http.Server
|
server *http.Server
|
||||||
|
|
||||||
|
mu sync.Mutex
|
||||||
|
closed bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewWebsocketListener to handle websocket connections
|
// NewWebsocketListener to handle websocket connections
|
||||||
@@ -41,7 +46,15 @@ func NewWebsocketListener(ln net.Listener) (wl *WebsocketListener) {
|
|||||||
conn := WrapCloseNotifyConn(c, func(_ error) {
|
conn := WrapCloseNotifyConn(c, func(_ error) {
|
||||||
close(notifyCh)
|
close(notifyCh)
|
||||||
})
|
})
|
||||||
|
// 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
|
wl.acceptCh <- conn
|
||||||
|
}); err != nil {
|
||||||
|
conn.Close()
|
||||||
|
return
|
||||||
|
}
|
||||||
<-notifyCh
|
<-notifyCh
|
||||||
}))
|
}))
|
||||||
|
|
||||||
@@ -66,6 +79,15 @@ func (p *WebsocketListener) Accept() (net.Conn, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (p *WebsocketListener) Close() 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()
|
return p.server.Close()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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")
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user