mirror of
https://github.com/go-gost/x.git
synced 2026-10-19 08:15:33 +00:00
fix(service): keep accepting after a conn limiter rejection
One connection over the climiter quota killed the whole service. The conn limiter wrapper signals an over-limit connection by returning a plain error from Accept, which the accept loop does not recognise as either a listener.AcceptError or a temporary net.Error, so it fell through to the fatal branch and returned. program.go discards Serve's return value, so the process stayed up with no further logging while nothing accepted connections any more. Every later connection sat in the listen backlog, where the kernel completes the handshake but no service ever reads it, so clients saw a connection that opens and then never negotiates. Only a restart recovered the service. The counter itself was never the problem: llimiter.Allow rolls the count back when it refuses, and a test now pins that. A test also pins that the accept loop survives a rejection and still serves the connection that follows it. Export ErrConnLimit so the refusal can be told apart from a listener that has genuinely failed, and continue the accept loop on it. The plain-http-tunnel listener refused over-limit connections with an identical error and is fixed the same way. Closes #914
This commit is contained in:
@@ -7,6 +7,13 @@ import (
|
||||
limiter "github.com/go-gost/core/limiter/conn"
|
||||
)
|
||||
|
||||
// ErrConnLimit reports that a connection was refused because it exceeded the
|
||||
// connection limit. It is a sentinel so callers can tell a single refused
|
||||
// connection apart from a listener that has genuinely failed: the refusal is
|
||||
// per-connection and leaves the listener usable, so it must not end an accept
|
||||
// loop.
|
||||
var ErrConnLimit = errors.New("connection limit exceeded")
|
||||
|
||||
type listener struct {
|
||||
net.Listener
|
||||
limiter limiter.ConnLimiter
|
||||
@@ -39,7 +46,7 @@ func (ln *listener) Accept() (net.Conn, error) {
|
||||
return WrapConn(lim, c), nil
|
||||
}
|
||||
c.Close()
|
||||
return nil, errors.New("connection limit exceeded")
|
||||
return nil, ErrConnLimit
|
||||
}
|
||||
|
||||
return c, nil
|
||||
|
||||
@@ -3,7 +3,6 @@ package wrapper
|
||||
import (
|
||||
"errors"
|
||||
"net"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -124,8 +123,10 @@ func TestAccept_LimitExceeded(t *testing.T) {
|
||||
if err == nil {
|
||||
t.Fatal("expected error when limit exceeded")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "limit exceeded") {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
// The rejection must be the sentinel so the service's accept loop can tell
|
||||
// a refused connection from a listener failure (gost#914).
|
||||
if !errors.Is(err, ErrConnLimit) {
|
||||
t.Fatalf("err = %v, want ErrConnLimit", err)
|
||||
}
|
||||
if c != nil {
|
||||
t.Fatal("conn should be nil when limit exceeded")
|
||||
@@ -135,6 +136,46 @@ func TestAccept_LimitExceeded(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// gost#914: a rejected connection must not consume a slot. Allow(1) is called
|
||||
// and then the connection is dropped, so the count has to come back down —
|
||||
// otherwise a single burst over the limit would wedge every later connection
|
||||
// even after the original ones closed.
|
||||
func TestAccept_RejectionDoesNotConsumeSlot(t *testing.T) {
|
||||
inner := &mockListener{conns: []net.Conn{&mockConn{}, &mockConn{}, &mockConn{}, &mockConn{}}}
|
||||
lim := &allowLimiter{limit: 1}
|
||||
cl := &connLimiter{lims: map[string]limiter.Limiter{"192.168.1.1": lim}}
|
||||
ln := WrapListener(cl, inner)
|
||||
|
||||
// Fill the only slot.
|
||||
c, err := ln.Accept()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Two rejections: the limiter must still hold exactly 1, not 2 or 3.
|
||||
for i := 0; i < 2; i++ {
|
||||
if _, err := ln.Accept(); !errors.Is(err, ErrConnLimit) {
|
||||
t.Fatalf("conn %d: err = %v, want ErrConnLimit", i, err)
|
||||
}
|
||||
}
|
||||
if lim.count != 1 {
|
||||
t.Fatalf("limiter count = %d, want 1 (rejections must not consume slots)", lim.count)
|
||||
}
|
||||
|
||||
// Closing the accepted connection releases the slot, so the next accept
|
||||
// succeeds — the state the service was stuck in before the fix.
|
||||
if err := c.Close(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if lim.count != 0 {
|
||||
t.Fatalf("limiter count = %d after close, want 0", lim.count)
|
||||
}
|
||||
|
||||
if _, err := ln.Accept(); err != nil {
|
||||
t.Fatalf("accept after the slot was released: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAccept_ListenerError(t *testing.T) {
|
||||
inner := &mockListener{} // no conns, Accept returns error
|
||||
cl := &connLimiter{lims: map[string]limiter.Limiter{}}
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
package pht
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"net"
|
||||
|
||||
"github.com/go-gost/core/limiter"
|
||||
@@ -125,7 +124,7 @@ func (l *phtListener) Accept() (conn net.Conn, err error) {
|
||||
if lim := l.options.ConnLimiter.Limiter(host); lim != nil {
|
||||
if !lim.Allow(1) {
|
||||
conn.Close()
|
||||
return nil, errors.New("connection limit exceeded")
|
||||
return nil, climiter.ErrConnLimit
|
||||
}
|
||||
conn = climiter.WrapConn(lim, conn)
|
||||
}
|
||||
|
||||
+14
-2
@@ -24,10 +24,11 @@ import (
|
||||
"github.com/go-gost/core/observer/stats"
|
||||
"github.com/go-gost/core/recorder"
|
||||
"github.com/go-gost/core/service"
|
||||
xctx "github.com/go-gost/x/ctx"
|
||||
xlogger "github.com/go-gost/x/logger"
|
||||
xbypass "github.com/go-gost/x/bypass"
|
||||
xctx "github.com/go-gost/x/ctx"
|
||||
connwrapper "github.com/go-gost/x/limiter/conn/wrapper"
|
||||
rate_limiter "github.com/go-gost/x/limiter/rate"
|
||||
xlogger "github.com/go-gost/x/logger"
|
||||
xmetrics "github.com/go-gost/x/metrics"
|
||||
xstats "github.com/go-gost/x/observer/stats"
|
||||
"github.com/google/shlex"
|
||||
@@ -231,6 +232,17 @@ func (s *defaultService) Serve() error {
|
||||
tempDelay = 0
|
||||
}
|
||||
|
||||
// A connection refused by the conn limiter is a per-connection
|
||||
// outcome, not a listener failure: that connection is already
|
||||
// closed and the listener is still usable. Returning it as fatal
|
||||
// ended the accept loop, so one rejection stopped the service for
|
||||
// good and every later connection was refused until a restart
|
||||
// (gost#914). Keep accepting.
|
||||
if errors.Is(e, connwrapper.ErrConnLimit) {
|
||||
log.Warnf("accept: %v", e)
|
||||
continue
|
||||
}
|
||||
|
||||
// TODO: remove Temporary checking
|
||||
if ne, ok := e.(net.Error); ok && ne.Temporary() {
|
||||
if tempDelay == 0 {
|
||||
|
||||
+49
-1
@@ -21,6 +21,7 @@ import (
|
||||
"github.com/go-gost/core/observer/stats"
|
||||
"github.com/go-gost/core/recorder"
|
||||
xctx "github.com/go-gost/x/ctx"
|
||||
connwrapper "github.com/go-gost/x/limiter/conn/wrapper"
|
||||
xlogger "github.com/go-gost/x/logger"
|
||||
)
|
||||
|
||||
@@ -235,6 +236,52 @@ func TestServeAcceptsConnection(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// gost#914: a single rejected connection must not end the accept loop. The conn
|
||||
// limiter's wrapper signals an over-limit connection by returning a plain error
|
||||
// from Accept, which the loop used to treat as fatal — so one rejection took the
|
||||
// whole service down and every later connection was refused until a restart.
|
||||
// The connection that was over the limit is refused; the service is not.
|
||||
func TestServeSurvivesRejectedConnection(t *testing.T) {
|
||||
ln := newMockListener()
|
||||
var handled atomic.Int32
|
||||
h := newMockHandler(func(ctx context.Context, conn net.Conn) error {
|
||||
handled.Add(1)
|
||||
return nil
|
||||
})
|
||||
svc := NewService("test", ln, h).(*defaultService)
|
||||
|
||||
done := make(chan error, 1)
|
||||
go func() { done <- svc.Serve() }()
|
||||
|
||||
newAddr := func(port int) *net.TCPAddr {
|
||||
return &net.TCPAddr{IP: net.ParseIP("192.168.1.1"), Port: port}
|
||||
}
|
||||
|
||||
// One connection is accepted, then the limiter rejects the next.
|
||||
ln.connCh <- &mockConn{remoteAddr: newAddr(1234), localAddr: ln.addr}
|
||||
ln.errCh <- connwrapper.ErrConnLimit
|
||||
|
||||
// The loop must still be running: a later connection has to be served.
|
||||
ln.connCh <- &mockConn{remoteAddr: newAddr(1235), localAddr: ln.addr}
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
t.Fatalf("accept loop exited after a rejected connection: %v", err)
|
||||
case <-time.After(200 * time.Millisecond):
|
||||
}
|
||||
|
||||
ln.Close()
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("accept loop did not exit after the listener closed")
|
||||
}
|
||||
|
||||
if got := handled.Load(); got != 2 {
|
||||
t.Errorf("handled = %d, want 2 (the connection after the rejection must be served)", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestServeSetsReadyState(t *testing.T) {
|
||||
ln := newMockListener()
|
||||
svc := NewService("test", ln, newMockHandler(nil)).(*defaultService)
|
||||
@@ -589,7 +636,8 @@ func TestCloseJoinsErrors(t *testing.T) {
|
||||
type closeErrHandler struct {
|
||||
err error
|
||||
}
|
||||
func (h *closeErrHandler) Init(metadata.Metadata) error { return nil }
|
||||
|
||||
func (h *closeErrHandler) Init(metadata.Metadata) error { return nil }
|
||||
func (h *closeErrHandler) Handle(context.Context, net.Conn, ...handler.HandleOption) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user