From 0ab5726a7aa5c2d66916b14becb696e6e19a1efb Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 19 Jul 2021 11:02:23 +0200 Subject: [PATCH] fix: start GRPCWebServer in goroutine (backport #9704) (#9719) * fix: start GRPCWebServer in goroutine (#9704) so it don't block other code from executing. Closes #9703 ## Description --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit d5674a5d3e82dcf47c4744af339a819e110e89f0) # Conflicts: # CHANGELOG.md * fix changelog Co-authored-by: yihuang Co-authored-by: Robert Zaremba --- CHANGELOG.md | 5 +++++ server/grpc/grpc_web.go | 17 +++++++++++++++-- server/grpc/server.go | 2 +- server/start.go | 4 ++-- server/types/app.go | 5 +++++ testutil/network/util.go | 18 ++++-------------- 6 files changed, 32 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35b40b99f091..f93d61b9de63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog +## [v0.43.0-rc2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc2) - 2021-07-19 + +### Bug Fixes + +* (server) [#9704](https://github.com/cosmos/cosmos-sdk/pull/9704) Start GRPCWebServer in goroutine, avoid blocking other services from starting. ## [v0.43.0-rc1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc1) - 2021-07-14 diff --git a/server/grpc/grpc_web.go b/server/grpc/grpc_web.go index 593779835a78..c2fc023a24f0 100644 --- a/server/grpc/grpc_web.go +++ b/server/grpc/grpc_web.go @@ -1,12 +1,15 @@ package grpc import ( + "fmt" "net/http" + "time" "github.com/improbable-eng/grpc-web/go/grpcweb" "google.golang.org/grpc" "github.com/cosmos/cosmos-sdk/server/config" + "github.com/cosmos/cosmos-sdk/server/types" ) // StartGRPCWeb starts a gRPC-Web server on the given address. @@ -28,8 +31,18 @@ func StartGRPCWeb(grpcSrv *grpc.Server, config config.Config) (*http.Server, err Addr: config.GRPCWeb.Address, Handler: http.HandlerFunc(handler), } - if err := grpcWebSrv.ListenAndServe(); err != nil { + + errCh := make(chan error) + go func() { + if err := grpcWebSrv.ListenAndServe(); err != nil { + errCh <- fmt.Errorf("[grpc] failed to serve: %w", err) + } + }() + + select { + case err := <-errCh: return nil, err + case <-time.After(types.ServerStartTime): // assume server started successfully + return grpcWebSrv, nil } - return grpcWebSrv, nil } diff --git a/server/grpc/server.go b/server/grpc/server.go index 5bc146d8d12d..40a3c7716d97 100644 --- a/server/grpc/server.go +++ b/server/grpc/server.go @@ -54,7 +54,7 @@ func StartGRPCServer(clientCtx client.Context, app types.Application, address st select { case err := <-errCh: return nil, err - case <-time.After(5 * time.Second): // assume server started successfully + case <-time.After(types.ServerStartTime): // assume server started successfully return grpcSrv, nil } } diff --git a/server/start.go b/server/start.go index 79e7bee28304..d26fc7ad7a7a 100644 --- a/server/start.go +++ b/server/start.go @@ -314,7 +314,7 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App select { case err := <-errCh: return err - case <-time.After(5 * time.Second): // assume server started successfully + case <-time.After(types.ServerStartTime): // assume server started successfully } } @@ -368,7 +368,7 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App select { case err := <-errCh: return err - case <-time.After(5 * time.Second): // assume server started successfully + case <-time.After(types.ServerStartTime): // assume server started successfully } } diff --git a/server/types/app.go b/server/types/app.go index ba702074b2a6..cf6b6ad9a1ff 100644 --- a/server/types/app.go +++ b/server/types/app.go @@ -3,6 +3,7 @@ package types import ( "encoding/json" "io" + "time" "github.com/gogo/protobuf/grpc" "github.com/spf13/cobra" @@ -16,6 +17,10 @@ import ( "github.com/cosmos/cosmos-sdk/server/config" ) +// ServerStartTime defines the time duration that the server need to stay running after startup +// for the startup be considered successful +const ServerStartTime = 5 * time.Second + type ( // AppOptions defines an interface that is passed into an application // constructor, typically used to set BaseApp options that are either supplied diff --git a/testutil/network/util.go b/testutil/network/util.go index c6a2bd815904..e7542113a87b 100644 --- a/testutil/network/util.go +++ b/testutil/network/util.go @@ -16,6 +16,7 @@ import ( "github.com/cosmos/cosmos-sdk/server/api" servergrpc "github.com/cosmos/cosmos-sdk/server/grpc" + srvtypes "github.com/cosmos/cosmos-sdk/server/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/cosmos/cosmos-sdk/x/genutil" @@ -90,7 +91,7 @@ func startInProcess(cfg Config, val *Validator) error { select { case err := <-errCh: return err - case <-time.After(5 * time.Second): // assume server started successfully + case <-time.After(srvtypes.ServerStartTime): // assume server started successfully } val.api = apiSrv @@ -105,21 +106,10 @@ func startInProcess(cfg Config, val *Validator) error { val.grpc = grpcSrv if val.AppConfig.GRPCWeb.Enable { - errCh1 := make(chan error) - go func() { - grpcWeb, err := servergrpc.StartGRPCWeb(grpcSrv, *val.AppConfig) - if err != nil { - errCh1 <- err - } - - val.grpcWeb = grpcWeb - }() - select { - case err := <-errCh1: + val.grpcWeb, err = servergrpc.StartGRPCWeb(grpcSrv, *val.AppConfig) + if err != nil { return err - case <-time.After(5 * time.Second): // assume server started successfully } - } }