Skip to content

Commit

Permalink
fix: race conditions in the plugin logic (#4091)
Browse files Browse the repository at this point in the history
* fix race condition for unit tests

* add changelog

* increase TestScaffoldedTests timeout

---------

Co-authored-by: Pantani <Pantani>
  • Loading branch information
Pantani authored and julienrbrt committed May 29, 2024
1 parent bce7055 commit 849f000
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 4 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
- [#3969](https://github.com/ignite/cli/pull/3969) Get first config validator using a getter to avoid index errors
- [#4000](https://github.com/ignite/cli/pull/4000) Run all dry runners before the wet run in the `xgenny` pkg
- [#4086](https://github.com/ignite/cli/pull/4086) Retry to get the IBC balance if it fails the first time
- [#4091](https://github.com/ignite/cli/pull/4091) Fix race conditions in the plugin logic
- [#4096](https://github.com/ignite/cli/pull/4096) Add new reserved names module and remove duplicated genesis order

## [`v28.3.0`](https://github.com/ignite/cli/releases/tag/v28.3.0)
Expand Down
3 changes: 2 additions & 1 deletion ignite/internal/plugin/execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,12 @@ func Execute(ctx context.Context, path string, args []string, options ...plugin.
if err != nil {
// Extract the rpc status message and create a simple error from it.
// We don't want Execute to return rpc errors.
err = errors.New(status.Convert(err).Message())
return "", errors.New(status.Convert(err).Message())
}
// NOTE(tb): This pause gives enough time for go-plugin to sync the
// output from stdout/stderr of the plugin. Without that pause, this
// output can be discarded and absent from buf.
time.Sleep(100 * time.Millisecond)
plugins[0].KillClient()
return buf.String(), err
}
6 changes: 6 additions & 0 deletions ignite/services/plugin/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package plugin

import (
"context"
"sync"

hplugin "github.com/hashicorp/go-plugin"
"google.golang.org/grpc"
Expand Down Expand Up @@ -108,19 +109,24 @@ func (c client) ExecuteHookCleanUp(ctx context.Context, h *ExecutedHook, api Cli
func (c client) startClientAPIServer(api ClientAPI) (uint32, func()) {
var (
srv *grpc.Server
m sync.Mutex
brokerID = c.broker.NextId()
)

go c.broker.AcceptAndServe(brokerID, func(opts []grpc.ServerOption) *grpc.Server {
m.Lock()
defer m.Unlock()
srv = grpc.NewServer(opts...)
v1.RegisterClientAPIServiceServer(srv, &clientAPIServer{impl: api})
return srv
})

stop := func() {
m.Lock()
if srv != nil {
srv.Stop()
}
m.Unlock()
}

return brokerID, stop
Expand Down
2 changes: 1 addition & 1 deletion ignite/services/plugin/scaffold_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestScaffoldedTests(t *testing.T) {
// Act
err := gocmd.Test(ctx, path, []string{
"-timeout",
"5m",
"10m",
"-run",
"^TestBar$",
})
Expand Down
2 changes: 1 addition & 1 deletion scripts/test-coverage
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash
set -e -x

go test -coverprofile=coverage.txt -covermode=atomic -coverpkg=./... $(go list github.com/ignite/cli/v29/ignite/...)
go test -race -coverprofile=coverage.txt -covermode=atomic -coverpkg=./... $(go list github.com/ignite/cli/v29/ignite/...)

# append "||true" to grep so if no match the return code stays 0
excludelist="$(find ./ -type f -name '*.go' | xargs grep -l 'DONTCOVER' || true)"
Expand Down
2 changes: 1 addition & 1 deletion scripts/test-integration
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#!/bin/bash

go test -v -timeout 120m github.com/ignite/cli/v29/integration/...
go test -v -timeout 30m github.com/ignite/cli/v29/integration/...

0 comments on commit 849f000

Please sign in to comment.