From 74d3a61039e15af43c9f67de0c164e6e894f8605 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 10 Dec 2024 09:48:09 +0800 Subject: [PATCH 1/7] fix(server/v2): avoid server stop get call before start for multi components init httpServer early to avoid nil httpServer when stop panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x1050bd0f0] goroutine 1 [running]: net/http.(*Server).Shutdown(0x10851b518?, {0x108560af8?, 0x14003124720?}) net/http/server.go:3050 +0x30 cosmossdk.io/server/v2/api/rest.(*Server[...]).Stop(0xd, {0x108560af8?, 0x14003124720}) cosmossdk.io/server/v2@v2.0.0-20240718121635-a877e3e8048a/api/rest/server.go:93 +0xe4 cosmossdk.io/server/v2.(*Server[...]).Stop(0x1085be3c0, {0x108560af8, 0x14003124720}) cosmossdk.io/server/v2@v2.0.0-20240718121635-a877e3e8048a/server.go:119 +0x21c cosmossdk.io/server/v2.createStartCommand[...].func1.2.1() cosmossdk.io/server/v2@v2.0.0-20240718121635-a877e3e8048a/commands.go:103 +0xdc cosmossdk.io/server/v2.createStartCommand[...].func1.2() cosmossdk.io/server/v2@v2.0.0-20240718121635-a877e3e8048a/co --- server/v2/api/rest/server.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/server/v2/api/rest/server.go b/server/v2/api/rest/server.go index 5cffaa282f69..1ea098c6dce0 100644 --- a/server/v2/api/rest/server.go +++ b/server/v2/api/rest/server.go @@ -46,7 +46,10 @@ func New[T transaction.Tx]( } } srv.config = serverCfg - + srv.httpServer = &http.Server{ + Addr: srv.config.Address, + Handler: srv.router, + } return srv, nil } @@ -69,11 +72,6 @@ func (s *Server[T]) Start(ctx context.Context) error { return nil } - s.httpServer = &http.Server{ - Addr: s.config.Address, - Handler: s.router, - } - s.logger.Info("starting HTTP server", "address", s.config.Address) if err := s.httpServer.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { s.logger.Error("failed to start HTTP server", "error", err) From 357ccfb8c94345efc80110ed7cf4df3f7c88a291 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 10 Dec 2024 09:49:20 +0800 Subject: [PATCH 2/7] reuse --- server/v2/api/rest/server.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/v2/api/rest/server.go b/server/v2/api/rest/server.go index 1ea098c6dce0..12fdb2a3b112 100644 --- a/server/v2/api/rest/server.go +++ b/server/v2/api/rest/server.go @@ -87,6 +87,12 @@ func (s *Server[T]) Stop(ctx context.Context) error { } s.logger.Info("stopping HTTP server") + defer func() { + s.httpServer = &http.Server{ + Addr: s.config.Address, + Handler: s.router, + } + }() return s.httpServer.Shutdown(ctx) } From 70751b6d0946842a313f79307864c5f234ddb0e0 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 10 Dec 2024 17:07:21 +0800 Subject: [PATCH 3/7] avoid failed to start gRPC-Gateway server on 1317 port conflict --- systemtests/system.go | 1 + 1 file changed, 1 insertion(+) diff --git a/systemtests/system.go b/systemtests/system.go index 5b964ee03327..fa39beb4d509 100644 --- a/systemtests/system.go +++ b/systemtests/system.go @@ -735,6 +735,7 @@ func (s *SystemUnderTest) AddFullnode(t *testing.T, beforeStart ...func(nodeNumb if tomlFile == "app.toml" && IsV2() { file := filepath.Join(WorkDir, s.nodePath(nodeNumber), "config", tomlFile) EditToml(file, func(doc *tomledit.Document) { + SetValue(doc, fmt.Sprintf("%s:%d", node.IP, DefaultApiPort+nodeNumber), "grpc-gateway", "address") SetValue(doc, fmt.Sprintf("%s:%d", node.IP, DefaultRestPort+nodeNumber), "rest", "address") }) } From 59bfdd939cbb67dbf6e5d2872cdfefd7b78e2473 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 10 Dec 2024 19:24:18 +0800 Subject: [PATCH 4/7] ref --- server/v2/api/rest/server.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/server/v2/api/rest/server.go b/server/v2/api/rest/server.go index 12fdb2a3b112..d5b8ddfccc8a 100644 --- a/server/v2/api/rest/server.go +++ b/server/v2/api/rest/server.go @@ -46,10 +46,7 @@ func New[T transaction.Tx]( } } srv.config = serverCfg - srv.httpServer = &http.Server{ - Addr: srv.config.Address, - Handler: srv.router, - } + srv.init() return srv, nil } @@ -62,6 +59,13 @@ func NewWithConfigOptions[T transaction.Tx](opts ...CfgOption) *Server[T] { } } +func (s *Server[T]) init() { + s.httpServer = &http.Server{ + Addr: s.config.Address, + Handler: s.router, + } +} + func (s *Server[T]) Name() string { return ServerName } @@ -87,12 +91,7 @@ func (s *Server[T]) Stop(ctx context.Context) error { } s.logger.Info("stopping HTTP server") - defer func() { - s.httpServer = &http.Server{ - Addr: s.config.Address, - Handler: s.router, - } - }() + defer s.init() return s.httpServer.Shutdown(ctx) } From 9db84ba50e2a3144120bef009c3299097ec703ff Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 10 Dec 2024 19:24:33 +0800 Subject: [PATCH 5/7] telemetry --- server/v2/api/telemetry/server.go | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/server/v2/api/telemetry/server.go b/server/v2/api/telemetry/server.go index f612964a8476..7efdeae16b79 100644 --- a/server/v2/api/telemetry/server.go +++ b/server/v2/api/telemetry/server.go @@ -52,9 +52,24 @@ func New[T transaction.Tx](cfg server.ConfigMap, logger log.Logger, enableTeleme return nil, fmt.Errorf("failed to initialize metrics: %w", err) } srv.metrics = metrics + srv.init() return srv, nil } +func (s *Server[T]) init() { + mux := http.NewServeMux() + // /metrics is the default standard path for Prometheus metrics. + mux.HandleFunc("/metrics", s.metricsHandler) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "/metrics", http.StatusMovedPermanently) + }) + + s.server = &http.Server{ + Addr: s.config.Address, + Handler: mux, + } +} + // Name returns the server name. func (s *Server[T]) Name() string { return ServerName @@ -74,18 +89,6 @@ func (s *Server[T]) Start(ctx context.Context) error { return nil } - mux := http.NewServeMux() - // /metrics is the default standard path for Prometheus metrics. - mux.HandleFunc("/metrics", s.metricsHandler) - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - http.Redirect(w, r, "/metrics", http.StatusMovedPermanently) - }) - - s.server = &http.Server{ - Addr: s.config.Address, - Handler: mux, - } - s.logger.Info("starting telemetry server...", "address", s.config.Address) if err := s.server.ListenAndServe(); err != nil && err != http.ErrServerClosed { return fmt.Errorf("failed to start telemetry server: %w", err) @@ -99,6 +102,7 @@ func (s *Server[T]) Stop(ctx context.Context) error { return nil } + defer s.init() s.logger.Info("stopping telemetry server...", "address", s.config.Address) return s.server.Shutdown(ctx) } From b69e5cd5f9bcc7585bd4cb611d30d25b9a97825c Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 10 Dec 2024 20:09:41 +0800 Subject: [PATCH 6/7] Apply suggestions from code review --- server/v2/api/rest/server.go | 13 ++++--------- server/v2/api/telemetry/server.go | 13 ++++--------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/server/v2/api/rest/server.go b/server/v2/api/rest/server.go index d5b8ddfccc8a..1ea098c6dce0 100644 --- a/server/v2/api/rest/server.go +++ b/server/v2/api/rest/server.go @@ -46,7 +46,10 @@ func New[T transaction.Tx]( } } srv.config = serverCfg - srv.init() + srv.httpServer = &http.Server{ + Addr: srv.config.Address, + Handler: srv.router, + } return srv, nil } @@ -59,13 +62,6 @@ func NewWithConfigOptions[T transaction.Tx](opts ...CfgOption) *Server[T] { } } -func (s *Server[T]) init() { - s.httpServer = &http.Server{ - Addr: s.config.Address, - Handler: s.router, - } -} - func (s *Server[T]) Name() string { return ServerName } @@ -91,7 +87,6 @@ func (s *Server[T]) Stop(ctx context.Context) error { } s.logger.Info("stopping HTTP server") - defer s.init() return s.httpServer.Shutdown(ctx) } diff --git a/server/v2/api/telemetry/server.go b/server/v2/api/telemetry/server.go index 7efdeae16b79..1236f9112ba0 100644 --- a/server/v2/api/telemetry/server.go +++ b/server/v2/api/telemetry/server.go @@ -52,22 +52,18 @@ func New[T transaction.Tx](cfg server.ConfigMap, logger log.Logger, enableTeleme return nil, fmt.Errorf("failed to initialize metrics: %w", err) } srv.metrics = metrics - srv.init() - return srv, nil -} - -func (s *Server[T]) init() { mux := http.NewServeMux() // /metrics is the default standard path for Prometheus metrics. - mux.HandleFunc("/metrics", s.metricsHandler) + mux.HandleFunc("/metrics", srv.metricsHandler) mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/metrics", http.StatusMovedPermanently) }) - s.server = &http.Server{ - Addr: s.config.Address, + srv.server = &http.Server{ + Addr: srv.config.Address, Handler: mux, } + return srv, nil } // Name returns the server name. @@ -102,7 +98,6 @@ func (s *Server[T]) Stop(ctx context.Context) error { return nil } - defer s.init() s.logger.Info("stopping telemetry server...", "address", s.config.Address) return s.server.Shutdown(ctx) } From dbe76eb4f78a74a6a3739706cf2e121262ac4c22 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 10 Dec 2024 20:10:02 +0800 Subject: [PATCH 7/7] grpcgateway --- server/v2/api/grpcgateway/server.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/server/v2/api/grpcgateway/server.go b/server/v2/api/grpcgateway/server.go index 16a622a94961..977eb369c894 100644 --- a/server/v2/api/grpcgateway/server.go +++ b/server/v2/api/grpcgateway/server.go @@ -75,7 +75,13 @@ func New[T transaction.Tx]( s.logger = logger.With(log.ModuleKey, s.Name()) s.config = serverCfg + mux := http.NewServeMux() + mux.Handle("/", s.GRPCGatewayRouter) + s.server = &http.Server{ + Addr: s.config.Address, + Handler: mux, + } return s, nil } @@ -110,14 +116,6 @@ func (s *Server[T]) Start(ctx context.Context) error { return nil } - mux := http.NewServeMux() - mux.Handle("/", s.GRPCGatewayRouter) - - s.server = &http.Server{ - Addr: s.config.Address, - Handler: mux, - } - s.logger.Info("starting gRPC-Gateway server...", "address", s.config.Address) if err := s.server.ListenAndServe(); err != nil && err != http.ErrServerClosed { return fmt.Errorf("failed to start gRPC-Gateway server: %w", err)