Skip to content

Commit 0ab5726

Browse files
mergify[bot]yihuangrobert-zaremba
authored
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 <!-- The default pull request template is for types feat, fix, or refactor. For other templates, add one of the following parameters to the url: - template=docs.md - template=other.md --> ## Description <!-- Add a description of the changes that this PR introduces and the files that are the most critical to review. --> --- ### 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 d5674a5) # Conflicts: # CHANGELOG.md * fix changelog Co-authored-by: yihuang <huang@crypto.com> Co-authored-by: Robert Zaremba <robert@zaremba.ch>
1 parent 606d56e commit 0ab5726

File tree

6 files changed

+32
-19
lines changed

6 files changed

+32
-19
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
3535

3636
# Changelog
3737

38+
## [v0.43.0-rc2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc2) - 2021-07-19
39+
40+
### Bug Fixes
41+
42+
* (server) [#9704](https://github.com/cosmos/cosmos-sdk/pull/9704) Start GRPCWebServer in goroutine, avoid blocking other services from starting.
3843

3944
## [v0.43.0-rc1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc1) - 2021-07-14
4045

server/grpc/grpc_web.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package grpc
22

33
import (
4+
"fmt"
45
"net/http"
6+
"time"
57

68
"github.com/improbable-eng/grpc-web/go/grpcweb"
79
"google.golang.org/grpc"
810

911
"github.com/cosmos/cosmos-sdk/server/config"
12+
"github.com/cosmos/cosmos-sdk/server/types"
1013
)
1114

1215
// 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
2831
Addr: config.GRPCWeb.Address,
2932
Handler: http.HandlerFunc(handler),
3033
}
31-
if err := grpcWebSrv.ListenAndServe(); err != nil {
34+
35+
errCh := make(chan error)
36+
go func() {
37+
if err := grpcWebSrv.ListenAndServe(); err != nil {
38+
errCh <- fmt.Errorf("[grpc] failed to serve: %w", err)
39+
}
40+
}()
41+
42+
select {
43+
case err := <-errCh:
3244
return nil, err
45+
case <-time.After(types.ServerStartTime): // assume server started successfully
46+
return grpcWebSrv, nil
3347
}
34-
return grpcWebSrv, nil
3548
}

server/grpc/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func StartGRPCServer(clientCtx client.Context, app types.Application, address st
5454
select {
5555
case err := <-errCh:
5656
return nil, err
57-
case <-time.After(5 * time.Second): // assume server started successfully
57+
case <-time.After(types.ServerStartTime): // assume server started successfully
5858
return grpcSrv, nil
5959
}
6060
}

server/start.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App
314314
select {
315315
case err := <-errCh:
316316
return err
317-
case <-time.After(5 * time.Second): // assume server started successfully
317+
case <-time.After(types.ServerStartTime): // assume server started successfully
318318
}
319319
}
320320

@@ -368,7 +368,7 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App
368368
select {
369369
case err := <-errCh:
370370
return err
371-
case <-time.After(5 * time.Second): // assume server started successfully
371+
case <-time.After(types.ServerStartTime): // assume server started successfully
372372
}
373373
}
374374

server/types/app.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package types
33
import (
44
"encoding/json"
55
"io"
6+
"time"
67

78
"github.com/gogo/protobuf/grpc"
89
"github.com/spf13/cobra"
@@ -16,6 +17,10 @@ import (
1617
"github.com/cosmos/cosmos-sdk/server/config"
1718
)
1819

20+
// ServerStartTime defines the time duration that the server need to stay running after startup
21+
// for the startup be considered successful
22+
const ServerStartTime = 5 * time.Second
23+
1924
type (
2025
// AppOptions defines an interface that is passed into an application
2126
// constructor, typically used to set BaseApp options that are either supplied

testutil/network/util.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/cosmos/cosmos-sdk/server/api"
1818
servergrpc "github.com/cosmos/cosmos-sdk/server/grpc"
19+
srvtypes "github.com/cosmos/cosmos-sdk/server/types"
1920
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
2021
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
2122
"github.com/cosmos/cosmos-sdk/x/genutil"
@@ -90,7 +91,7 @@ func startInProcess(cfg Config, val *Validator) error {
9091
select {
9192
case err := <-errCh:
9293
return err
93-
case <-time.After(5 * time.Second): // assume server started successfully
94+
case <-time.After(srvtypes.ServerStartTime): // assume server started successfully
9495
}
9596

9697
val.api = apiSrv
@@ -105,21 +106,10 @@ func startInProcess(cfg Config, val *Validator) error {
105106
val.grpc = grpcSrv
106107

107108
if val.AppConfig.GRPCWeb.Enable {
108-
errCh1 := make(chan error)
109-
go func() {
110-
grpcWeb, err := servergrpc.StartGRPCWeb(grpcSrv, *val.AppConfig)
111-
if err != nil {
112-
errCh1 <- err
113-
}
114-
115-
val.grpcWeb = grpcWeb
116-
}()
117-
select {
118-
case err := <-errCh1:
109+
val.grpcWeb, err = servergrpc.StartGRPCWeb(grpcSrv, *val.AppConfig)
110+
if err != nil {
119111
return err
120-
case <-time.After(5 * time.Second): // assume server started successfully
121112
}
122-
123113
}
124114
}
125115

0 commit comments

Comments
 (0)