You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by xu...@apache.org on 2026/08/20 05:13:15 UTC

(dubbo-go) branch develop updated: fix(triple): filter http.ErrServerClosed in single-protocol server shutdown (#3671)

This is an automated email from the ASF dual-hosted git repository.

Alanxtl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git


The following commit(s) were added to refs/heads/develop by this push:
     new da5838678 fix(triple): filter http.ErrServerClosed in single-protocol server shutdown (#3671)
da5838678 is described below

commit da5838678668437077f09e55f7f9fac9697a6c3c
Author: Li Zining <li...@outlook.com>
AuthorDate: Thu Aug 20 13:13:11 2026 +0800

    fix(triple): filter http.ErrServerClosed in single-protocol server shutdown (#3671)
    
    * fix(triple): filter http.ErrServerClosed in single-protocol server shutdown
    
    startHttp2 and startHttp3 returned the net/http serve error verbatim, so a normal Stop surfaced http.ErrServerClosed from Server.Run and the outer startTransport goroutine logged a spurious "server serve failed" error. The dual-protocol startHttp2AndHttp3 already swallowed this normal-shutdown signal inside its errgroup. Filter it in both single-protocol paths so Run returns nil after a clean shutdown, matching the dual-protocol behavior. Only http.ErrServerClosed is swallowed; genuin [...]
    
    Fixes: #3670
    Signed-off-by: lizining <li...@outlook.com>
    
    * test(triple): assert clean shutdown in single-protocol lifecycle tests
    
    The StartAndStop tests previously asserted that Run returns http.ErrServerClosed after a normal Stop, pinning the leak as expected behavior. Switch the six assertions to NoError and fold the now-uniform if/else in TestServer_RepeatedStartStop into a single assertion. Add TestServerRunReturnsBindErrorWhenPortInUse and TestServerRunHTTP3ReturnsBindErrorWhenPortInUse, which assert that genuine serve errors (port conflicts) still propagate, guarding the filter boundary. They fail on the p [...]
    
    Signed-off-by: lizining <li...@outlook.com>
    
    ---------
    
    Signed-off-by: lizining <li...@outlook.com>
---
 protocol/triple/triple_protocol/server.go          | 12 ++++--
 .../triple_protocol/server_lifecycle_test.go       | 50 +++++++++++++++-------
 2 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/protocol/triple/triple_protocol/server.go b/protocol/triple/triple_protocol/server.go
index 10cde267a..41b12edb8 100644
--- a/protocol/triple/triple_protocol/server.go
+++ b/protocol/triple/triple_protocol/server.go
@@ -215,8 +215,10 @@ func (s *Server) startHttp2(tlsConf *tls.Config) error {
 	} else {
 		err = srv.ListenAndServe()
 	}
-
-	return err
+	if err != nil && err != http.ErrServerClosed {
+		return err
+	}
+	return nil
 }
 
 func (s *Server) startHttp3(tlsConf *tls.Config) error {
@@ -246,7 +248,11 @@ func (s *Server) startHttp3(tlsConf *tls.Config) error {
 
 	logger.Debugf("[Triple][Server] triple HTTP/3 Server starting on %v", s.addr)
 
-	return s.http3Srv.Load().ListenAndServe()
+	err = s.http3Srv.Load().ListenAndServe()
+	if err != nil && err != http.ErrServerClosed {
+		return err
+	}
+	return nil
 }
 
 func (s *Server) startHttp2AndHttp3(tlsConf *tls.Config) error {
diff --git a/protocol/triple/triple_protocol/server_lifecycle_test.go b/protocol/triple/triple_protocol/server_lifecycle_test.go
index 2e6c39d4d..25b7a4442 100644
--- a/protocol/triple/triple_protocol/server_lifecycle_test.go
+++ b/protocol/triple/triple_protocol/server_lifecycle_test.go
@@ -28,7 +28,6 @@ import (
 	"errors"
 	"math/big"
 	"net"
-	"net/http"
 	"syscall"
 	"testing"
 	"time"
@@ -175,7 +174,7 @@ func TestServer_HTTP2_StartAndStop(t *testing.T) {
 	require.Nil(t, srv.http3Srv.Load())
 
 	require.NoError(t, srv.Stop())
-	require.ErrorIs(t, waitForServerExit(t, errCh, 5*time.Second), http.ErrServerClosed)
+	require.NoError(t, waitForServerExit(t, errCh, 5*time.Second))
 }
 
 func TestServer_HTTP2_StartAndStopWithTLS(t *testing.T) {
@@ -186,7 +185,7 @@ func TestServer_HTTP2_StartAndStopWithTLS(t *testing.T) {
 	require.Nil(t, srv.http3Srv.Load())
 
 	require.NoError(t, srv.Stop())
-	require.ErrorIs(t, waitForServerExit(t, errCh, 5*time.Second), http.ErrServerClosed)
+	require.NoError(t, waitForServerExit(t, errCh, 5*time.Second))
 }
 
 func TestServer_HTTP3_StartAndStop(t *testing.T) {
@@ -200,7 +199,7 @@ func TestServer_HTTP3_StartAndStop(t *testing.T) {
 	require.Nil(t, srv.httpSrv.Load())
 
 	require.NoError(t, srv.Stop())
-	require.ErrorIs(t, waitForServerExit(t, errCh, 5*time.Second), http.ErrServerClosed)
+	require.NoError(t, waitForServerExit(t, errCh, 5*time.Second))
 }
 
 func TestServer_HTTP2AndHTTP3_StartAndStop(t *testing.T) {
@@ -215,8 +214,6 @@ func TestServer_HTTP2AndHTTP3_StartAndStop(t *testing.T) {
 	require.NotNil(t, srv.http3Srv.Load())
 
 	require.NoError(t, srv.Stop())
-	// startHttp2AndHttp3 swallows http.ErrServerClosed inside the errgroup,
-	// so Run returns nil after the servers are closed.
 	require.NoError(t, waitForServerExit(t, errCh, 5*time.Second))
 }
 
@@ -230,7 +227,7 @@ func TestServer_HTTP2_StartAndGracefulStop(t *testing.T) {
 	graceCtx, cancel := context.WithTimeout(context.Background(), constant.DefaultGracefulShutdownTimeout)
 	defer cancel()
 	require.NoError(t, srv.GracefulStop(graceCtx))
-	require.ErrorIs(t, waitForServerExit(t, errCh, 5*time.Second), http.ErrServerClosed)
+	require.NoError(t, waitForServerExit(t, errCh, 5*time.Second))
 }
 
 func TestServer_HTTP3_StartAndGracefulStop(t *testing.T) {
@@ -246,7 +243,7 @@ func TestServer_HTTP3_StartAndGracefulStop(t *testing.T) {
 	graceCtx, cancel := context.WithTimeout(context.Background(), constant.DefaultGracefulShutdownTimeout)
 	defer cancel()
 	require.NoError(t, srv.GracefulStop(graceCtx))
-	require.ErrorIs(t, waitForServerExit(t, errCh, 5*time.Second), http.ErrServerClosed)
+	require.NoError(t, waitForServerExit(t, errCh, 5*time.Second))
 }
 
 func TestServer_HTTP2AndHTTP3_StartAndGracefulStop(t *testing.T) {
@@ -263,8 +260,6 @@ func TestServer_HTTP2AndHTTP3_StartAndGracefulStop(t *testing.T) {
 	graceCtx, cancel := context.WithTimeout(context.Background(), constant.DefaultGracefulShutdownTimeout)
 	defer cancel()
 	require.NoError(t, srv.GracefulStop(graceCtx))
-	// startHttp2AndHttp3 swallows http.ErrServerClosed inside the errgroup,
-	// so Run returns nil after the servers are closed.
 	require.NoError(t, waitForServerExit(t, errCh, 5*time.Second))
 }
 
@@ -334,11 +329,36 @@ func TestServer_RepeatedStartStop(t *testing.T) {
 			}
 
 			require.NoError(t, srv.Stop())
-			if tc.protocol == constant.CallHTTP2AndHTTP3 {
-				require.NoError(t, waitForServerExit(t, errCh, 5*time.Second))
-			} else {
-				require.ErrorIs(t, waitForServerExit(t, errCh, 5*time.Second), http.ErrServerClosed)
-			}
+			require.NoError(t, waitForServerExit(t, errCh, 5*time.Second))
 		}
 	}
 }
+
+// TestServerRunReturnsBindErrorWhenPortInUse verifies that Run propagates a
+// genuine serve error (here a TCP port conflict) instead of swallowing it
+// together with http.ErrServerClosed. It guards the boundary of the shutdown
+// filter: only the normal-closure signal must be suppressed.
+func TestServerRunReturnsBindErrorWhenPortInUse(t *testing.T) {
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	defer l.Close()
+
+	srv := NewServer(l.Addr().String(), &global.TripleConfig{})
+	err = srv.Run(constant.CallHTTP2, nil)
+	require.Error(t, err)
+	require.True(t, isAddrInUse(err), "expected EADDRINUSE, got %v", err)
+}
+
+// TestServerRunHTTP3ReturnsBindErrorWhenPortInUse verifies the same boundary
+// for the HTTP/3 path, where the QUIC endpoint fails to bind the occupied
+// UDP port.
+func TestServerRunHTTP3ReturnsBindErrorWhenPortInUse(t *testing.T) {
+	pc, err := net.ListenPacket("udp", "127.0.0.1:0")
+	require.NoError(t, err)
+	defer pc.Close()
+
+	srv := NewServer(pc.LocalAddr().String(), &global.TripleConfig{})
+	err = srv.Run(constant.CallHTTP3, newTestTLSConfig(t))
+	require.Error(t, err)
+	require.True(t, isAddrInUse(err), "expected EADDRINUSE, got %v", err)
+}