fix: race conditions in the plugin logic (#4091)

* fix race condition for unit tests

* add changelog

* increase TestScaffoldedTests timeout

---------

Co-authored-by: Pantani <Pantani>

Author: Pantani

changelog.md

@@ -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)

ignite/internal/plugin/execute.go

@@ -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
 }

ignite/services/plugin/protocol.go

@@ -2,6 +2,7 @@ package plugin
 
 import (
 	"context"
+	"sync"
 
 	hplugin "github.com/hashicorp/go-plugin"
 	"google.golang.org/grpc"
@@ -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

ignite/services/plugin/scaffold_test.go

@@ -52,7 +52,7 @@ func TestScaffoldedTests(t *testing.T) {
 	// Act
 	err := gocmd.Test(ctx, path, []string{
 		"-timeout",
-		"5m",
+		"10m",
 		"-run",
 		"^TestBar$",
 	})

scripts/test-coverage

@@ -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)"

scripts/test-integration

@@ -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/...