From 8c99444767cf40b908277fb9dd52902d2b337200 Mon Sep 17 00:00:00 2001 From: "Kim, JinSan" Date: Mon, 28 Dec 2020 17:47:43 +0900 Subject: [PATCH] test: grpc_client_test and fix grpc_client `StopForError()` hang bug --- abci/client/grpc_client.go | 2 +- abci/client/grpc_client_test.go | 93 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 abci/client/grpc_client_test.go diff --git a/abci/client/grpc_client.go b/abci/client/grpc_client.go index 01583bc1f..c79278846 100644 --- a/abci/client/grpc_client.go +++ b/abci/client/grpc_client.go @@ -88,11 +88,11 @@ func (cli *grpcClient) OnStop() { } func (cli *grpcClient) StopForError(err error) { - cli.mtx.Lock() if !cli.IsRunning() { return } + cli.mtx.Lock() if cli.err == nil { cli.err = err } diff --git a/abci/client/grpc_client_test.go b/abci/client/grpc_client_test.go new file mode 100644 index 000000000..aba6662ec --- /dev/null +++ b/abci/client/grpc_client_test.go @@ -0,0 +1,93 @@ +package abcicli_test + +import ( + "errors" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + abcicli "github.com/tendermint/tendermint/abci/client" + "github.com/tendermint/tendermint/abci/server" + "github.com/tendermint/tendermint/abci/types" + tmrand "github.com/tendermint/tendermint/libs/rand" + "github.com/tendermint/tendermint/libs/service" +) + +type errorStopper interface { + StopForError(error) +} + +func TestSocketClientStopForErrorDeadlock(t *testing.T) { + c := abcicli.NewGRPCClient(":80", false).(errorStopper) + err := errors.New("foo-tendermint") + + // See Issue https://github.com/tendermint/abci/issues/114 + doneChan := make(chan bool) + go func() { + defer close(doneChan) + c.StopForError(err) + c.StopForError(err) + }() + + select { + case <-doneChan: + case <-time.After(time.Second * 4): + t.Fatalf("Test took too long, potential deadlock still exists") + } +} + +func TestProperSyncCalls(t *testing.T) { + app := slowApp{} + + s, c := setupClientServer(t, app) + defer s.Stop() + defer c.Stop() + + resp := make(chan error, 1) + go func() { + // This is BeginBlockSync unrolled.... + reqres := c.BeginBlockAsync(types.RequestBeginBlock{}) + c.FlushSync() + res := reqres.Response.GetBeginBlock() + require.NotNil(t, res) + resp <- c.Error() + }() + + select { + case <-time.After(time.Second): + require.Fail(t, "No response arrived") + case err, ok := <-resp: + require.True(t, ok, "Must not close channel") + assert.NoError(t, err, "This should return success") + } +} + +func setupClientServer(t *testing.T, app types.Application) ( + service.Service, abcicli.Client) { + // some port between 20k and 30k + port := 20000 + tmrand.Int32()%10000 + addr := fmt.Sprintf("localhost:%d", port) + + s, err := server.NewServer(addr, "grpc", app) + require.NoError(t, err) + err = s.Start() + require.NoError(t, err) + + c := abcicli.NewGRPCClient(addr, true) + err = c.Start() + require.NoError(t, err) + + return s, c +} + +type slowApp struct { + types.BaseApplication +} + +func (slowApp) BeginBlock(req types.RequestBeginBlock) types.ResponseBeginBlock { + time.Sleep(200 * time.Millisecond) + return types.ResponseBeginBlock{} +}