From 00551e97789afe456aa281fc6e23c78c1c6d09dd Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 25 Oct 2023 12:09:03 +0200 Subject: [PATCH 1/4] refactor(network): call `app.Close()` on network cleanup (#18249) (cherry picked from commit 139a29e7e204d6848c5d4a5ed2472597aa939661) # Conflicts: # testutil/network/network.go # testutil/network/util.go --- testutil/network/network.go | 19 ++++++++++++++-- testutil/network/util.go | 45 +++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/testutil/network/network.go b/testutil/network/network.go index 0724a97f3b3a..b7db7d201d11 100644 --- a/testutil/network/network.go +++ b/testutil/network/network.go @@ -240,10 +240,20 @@ type ( ValAddress sdk.ValAddress RPCClient tmclient.Client +<<<<<<< HEAD tmNode *node.Node api *api.Server grpc *grpc.Server grpcWeb *http.Server +======= + app servertypes.Application + tmNode *node.Node + api *api.Server + grpc *grpc.Server + grpcWeb *http.Server + errGroup *errgroup.Group + cancelFn context.CancelFunc +>>>>>>> 139a29e7e (refactor(network): call `app.Close()` on network cleanup (#18249)) } // ValidatorI expose a validator's context and configuration @@ -581,8 +591,7 @@ func New(l Logger, baseDir string, cfg Config) (*Network, error) { l.Log("starting test network...") for idx, v := range network.Validators { - err := startInProcess(cfg, v) - if err != nil { + if err := startInProcess(cfg, v); err != nil { return nil, err } l.Log("started validator", idx) @@ -734,6 +743,12 @@ func (n *Network) Cleanup() { _ = v.grpcWeb.Close() } } + + if v.app != nil { + if err := v.app.Close(); err != nil { + n.Logger.Log("failed to stop validator ABCI application", "err", err) + } + } } // Give a brief pause for things to finish closing in other processes. Hopefully this helps with the address-in-use errors. diff --git a/testutil/network/util.go b/testutil/network/util.go index 24ad548f9418..d09063fdf7f4 100644 --- a/testutil/network/util.go +++ b/testutil/network/util.go @@ -15,6 +15,12 @@ import ( "github.com/cometbft/cometbft/types" tmtime "github.com/cometbft/cometbft/types/time" +<<<<<<< HEAD +======= + "cosmossdk.io/log" + + "github.com/cosmos/cosmos-sdk/server" +>>>>>>> 139a29e7e (refactor(network): call `app.Close()` on network cleanup (#18249)) "github.com/cosmos/cosmos-sdk/server/api" servergrpc "github.com/cosmos/cosmos-sdk/server/grpc" srvtypes "github.com/cosmos/cosmos-sdk/server/types" @@ -39,7 +45,17 @@ func startInProcess(cfg Config, val *Validator) error { } app := cfg.AppConstructor(*val) +<<<<<<< HEAD genDocProvider := node.DefaultGenesisDocProviderFunc(tmCfg) +======= + val.app = app + + appGenesisProvider := func() (*cmttypes.GenesisDoc, error) { + appGenesis, err := genutiltypes.AppGenesisFromFile(cmtCfg.GenesisFile()) + if err != nil { + return nil, err + } +>>>>>>> 139a29e7e (refactor(network): call `app.Close()` on network cleanup (#18249)) tmNode, err := node.NewNode( //resleak:notresource tmCfg, @@ -71,11 +87,40 @@ func startInProcess(cfg Config, val *Validator) error { app.RegisterTxService(val.ClientCtx) app.RegisterTendermintService(val.ClientCtx) +<<<<<<< HEAD app.RegisterNodeService(val.ClientCtx) } if val.APIAddress != "" { apiSrv := api.New(val.ClientCtx, logger.With("module", "api-server")) +======= + app.RegisterNodeService(val.ClientCtx, *val.AppConfig) + } + + ctx := context.Background() + ctx, val.cancelFn = context.WithCancel(ctx) + val.errGroup, ctx = errgroup.WithContext(ctx) + + grpcCfg := val.AppConfig.GRPC + + if grpcCfg.Enable { + grpcSrv, err := servergrpc.NewGRPCServer(val.ClientCtx, app, grpcCfg) + if err != nil { + return err + } + + // Start the gRPC server in a goroutine. Note, the provided ctx will ensure + // that the server is gracefully shut down. + val.errGroup.Go(func() error { + return servergrpc.StartGRPCServer(ctx, logger.With(log.ModuleKey, "grpc-server"), grpcCfg, grpcSrv) + }) + + val.grpc = grpcSrv + } + + if val.APIAddress != "" { + apiSrv := api.New(val.ClientCtx, logger.With(log.ModuleKey, "api-server"), val.grpc) +>>>>>>> 139a29e7e (refactor(network): call `app.Close()` on network cleanup (#18249)) app.RegisterAPIRoutes(apiSrv, val.AppConfig.API) errCh := make(chan error) From db0c9834d4a7219cb217888d68e8dbd9d4378116 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 25 Oct 2023 14:06:38 +0200 Subject: [PATCH 2/4] fix conflicts --- testutil/network/network.go | 11 +-------- testutil/network/util.go | 46 +------------------------------------ 2 files changed, 2 insertions(+), 55 deletions(-) diff --git a/testutil/network/network.go b/testutil/network/network.go index b7db7d201d11..ef065d9e68b8 100644 --- a/testutil/network/network.go +++ b/testutil/network/network.go @@ -240,20 +240,11 @@ type ( ValAddress sdk.ValAddress RPCClient tmclient.Client -<<<<<<< HEAD + app servertypes.Application tmNode *node.Node api *api.Server grpc *grpc.Server grpcWeb *http.Server -======= - app servertypes.Application - tmNode *node.Node - api *api.Server - grpc *grpc.Server - grpcWeb *http.Server - errGroup *errgroup.Group - cancelFn context.CancelFunc ->>>>>>> 139a29e7e (refactor(network): call `app.Close()` on network cleanup (#18249)) } // ValidatorI expose a validator's context and configuration diff --git a/testutil/network/util.go b/testutil/network/util.go index d09063fdf7f4..e67a96f4433f 100644 --- a/testutil/network/util.go +++ b/testutil/network/util.go @@ -15,12 +15,6 @@ import ( "github.com/cometbft/cometbft/types" tmtime "github.com/cometbft/cometbft/types/time" -<<<<<<< HEAD -======= - "cosmossdk.io/log" - - "github.com/cosmos/cosmos-sdk/server" ->>>>>>> 139a29e7e (refactor(network): call `app.Close()` on network cleanup (#18249)) "github.com/cosmos/cosmos-sdk/server/api" servergrpc "github.com/cosmos/cosmos-sdk/server/grpc" srvtypes "github.com/cosmos/cosmos-sdk/server/types" @@ -45,17 +39,8 @@ func startInProcess(cfg Config, val *Validator) error { } app := cfg.AppConstructor(*val) -<<<<<<< HEAD - genDocProvider := node.DefaultGenesisDocProviderFunc(tmCfg) -======= val.app = app - - appGenesisProvider := func() (*cmttypes.GenesisDoc, error) { - appGenesis, err := genutiltypes.AppGenesisFromFile(cmtCfg.GenesisFile()) - if err != nil { - return nil, err - } ->>>>>>> 139a29e7e (refactor(network): call `app.Close()` on network cleanup (#18249)) + genDocProvider := node.DefaultGenesisDocProviderFunc(tmCfg) tmNode, err := node.NewNode( //resleak:notresource tmCfg, @@ -87,40 +72,11 @@ func startInProcess(cfg Config, val *Validator) error { app.RegisterTxService(val.ClientCtx) app.RegisterTendermintService(val.ClientCtx) -<<<<<<< HEAD app.RegisterNodeService(val.ClientCtx) } if val.APIAddress != "" { apiSrv := api.New(val.ClientCtx, logger.With("module", "api-server")) -======= - app.RegisterNodeService(val.ClientCtx, *val.AppConfig) - } - - ctx := context.Background() - ctx, val.cancelFn = context.WithCancel(ctx) - val.errGroup, ctx = errgroup.WithContext(ctx) - - grpcCfg := val.AppConfig.GRPC - - if grpcCfg.Enable { - grpcSrv, err := servergrpc.NewGRPCServer(val.ClientCtx, app, grpcCfg) - if err != nil { - return err - } - - // Start the gRPC server in a goroutine. Note, the provided ctx will ensure - // that the server is gracefully shut down. - val.errGroup.Go(func() error { - return servergrpc.StartGRPCServer(ctx, logger.With(log.ModuleKey, "grpc-server"), grpcCfg, grpcSrv) - }) - - val.grpc = grpcSrv - } - - if val.APIAddress != "" { - apiSrv := api.New(val.ClientCtx, logger.With(log.ModuleKey, "api-server"), val.grpc) ->>>>>>> 139a29e7e (refactor(network): call `app.Close()` on network cleanup (#18249)) app.RegisterAPIRoutes(apiSrv, val.AppConfig.API) errCh := make(chan error) From 5f1fe25cea56c628469e271f1b82164e1366810d Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 25 Oct 2023 14:12:41 +0200 Subject: [PATCH 3/4] move up defer and call close even if tm not running --- server/start.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/server/start.go b/server/start.go index 31deb30745dc..aa8f5c47734b 100644 --- a/server/start.go +++ b/server/start.go @@ -304,7 +304,6 @@ func startStandAlone(ctx *Context, clientCtx client.Context, appCreator types.Ap defer func() { _ = svr.Stop() - _ = app.Close() if apiSrv != nil { @@ -437,6 +436,24 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App } } + defer func() { + if tmNode != nil && tmNode.IsRunning() { + _ = tmNode.Stop() + } + + if traceWriterCleanup != nil { + traceWriterCleanup() + } + + _ = app.Close() + + if apiSrv != nil { + _ = apiSrv.Close() + } + + ctx.Logger.Info("exiting...") + }() + // At this point it is safe to block the process if we're in gRPC only mode as // we do not need to start Rosetta or handle any Tendermint related processes. if gRPCOnly { @@ -494,23 +511,6 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App } } - defer func() { - if tmNode != nil && tmNode.IsRunning() { - _ = tmNode.Stop() - _ = app.Close() - } - - if traceWriterCleanup != nil { - traceWriterCleanup() - } - - if apiSrv != nil { - _ = apiSrv.Close() - } - - ctx.Logger.Info("exiting...") - }() - // wait for signal capture and gracefully return return WaitForQuitSignals() } From e81c669e0ccd68619898497eb8f261acbc287ea5 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 25 Oct 2023 14:14:52 +0200 Subject: [PATCH 4/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b496daa03093..3d99941e53d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (server) [#18251](https://github.com/cosmos/cosmos-sdk/pull/18251) Call `baseapp.Close()` when app started as grpc only. * (baseapp) [#17769](https://github.com/cosmos/cosmos-sdk/pull/17769) Ensure we respect block size constraints in the `DefaultProposalHandler`'s `PrepareProposal` handler when a nil or no-op mempool is used. We provide a `TxSelector` type to assist in making transaction selection generalized. We also fix a comparison bug in tx selection when `req.maxTxBytes` is reached. * (config) [#17649](https://github.com/cosmos/cosmos-sdk/pull/17649) Fix `mempool.max-txs` configuration is invalid in `app.config`. * (mempool) [#17668](https://github.com/cosmos/cosmos-sdk/pull/17668) Fix `PriorityNonceIterator.Next()` nil pointer ref for min priority at the end of iteration.