apply patch

Author: dmyger

shutdown_test.go

@@ -129,18 +129,16 @@ func testGracefulShutdown(t *testing.T, conn *Connection, inst *test_helpers.Tar
 func TestGracefulShutdown(t *testing.T) {
 	test_helpers.SkipIfWatchersUnsupported(t)
 
-	var inst test_helpers.TarantoolInstance
 	var conn *Connection
-	var err error
 
-	inst, err = test_helpers.StartTarantool(shtdnSrvOpts)
+	inst, err := test_helpers.StartTarantool(shtdnSrvOpts)
 	require.Nil(t, err)
 	defer test_helpers.StopTarantoolWithCleanup(inst)
 
 	conn = test_helpers.ConnectWithValidation(t, shtdnDialer, shtdnClntOpts)
 	defer conn.Close()
 
-	testGracefulShutdown(t, conn, &inst)
+	testGracefulShutdown(t, conn, inst)
 }
 
 func TestCloseGraceful(t *testing.T) {
@@ -190,26 +188,23 @@ func TestCloseGraceful(t *testing.T) {
 func TestGracefulShutdownWithReconnect(t *testing.T) {
 	test_helpers.SkipIfWatchersUnsupported(t)
 
-	var inst test_helpers.TarantoolInstance
-	var err error
-
-	inst, err = test_helpers.StartTarantool(shtdnSrvOpts)
+	inst, err := test_helpers.StartTarantool(shtdnSrvOpts)
 	require.Nil(t, err)
 	defer test_helpers.StopTarantoolWithCleanup(inst)
 
 	conn := test_helpers.ConnectWithValidation(t, shtdnDialer, shtdnClntOpts)
 	defer conn.Close()
 
-	testGracefulShutdown(t, conn, &inst)
+	testGracefulShutdown(t, conn, inst)
 
-	err = test_helpers.RestartTarantool(&inst)
+	err = test_helpers.RestartTarantool(inst)
 	require.Nilf(t, err, "Failed to restart tarantool")
 
 	connected := test_helpers.WaitUntilReconnected(conn, shtdnClntOpts.MaxReconnects,
 		shtdnClntOpts.Reconnect)
 	require.Truef(t, connected, "Reconnect success")
 
-	testGracefulShutdown(t, conn, &inst)
+	testGracefulShutdown(t, conn, inst)
 }
 
 func TestNoGracefulShutdown(t *testing.T) {
@@ -219,14 +214,12 @@ func TestNoGracefulShutdown(t *testing.T) {
 	noShtdDialer.RequiredProtocolInfo = ProtocolInfo{}
 	test_helpers.SkipIfWatchersSupported(t)
 
-	var inst test_helpers.TarantoolInstance
 	var conn *Connection
-	var err error
 
 	testSrvOpts := shtdnSrvOpts
 	testSrvOpts.Dialer = noShtdDialer
 
-	inst, err = test_helpers.StartTarantool(testSrvOpts)
+	inst, err := test_helpers.StartTarantool(testSrvOpts)
 	require.Nil(t, err)
 	defer test_helpers.StopTarantoolWithCleanup(inst)
 
@@ -274,11 +267,9 @@ func TestNoGracefulShutdown(t *testing.T) {
 func TestGracefulShutdownRespectsClose(t *testing.T) {
 	test_helpers.SkipIfWatchersUnsupported(t)
 
-	var inst test_helpers.TarantoolInstance
 	var conn *Connection
-	var err error
 
-	inst, err = test_helpers.StartTarantool(shtdnSrvOpts)
+	inst, err := test_helpers.StartTarantool(shtdnSrvOpts)
 	require.Nil(t, err)
 	defer test_helpers.StopTarantoolWithCleanup(inst)
 
@@ -354,11 +345,9 @@ func TestGracefulShutdownRespectsClose(t *testing.T) {
 func TestGracefulShutdownNotRacesWithRequestReconnect(t *testing.T) {
 	test_helpers.SkipIfWatchersUnsupported(t)
 
-	var inst test_helpers.TarantoolInstance
 	var conn *Connection
-	var err error
 
-	inst, err = test_helpers.StartTarantool(shtdnSrvOpts)
+	inst, err := test_helpers.StartTarantool(shtdnSrvOpts)
 	require.Nil(t, err)
 	defer test_helpers.StopTarantoolWithCleanup(inst)
 
@@ -425,11 +414,9 @@ func TestGracefulShutdownNotRacesWithRequestReconnect(t *testing.T) {
 func TestGracefulShutdownCloseConcurrent(t *testing.T) {
 	test_helpers.SkipIfWatchersUnsupported(t)
 
-	var inst test_helpers.TarantoolInstance
-	var err error
 	var srvShtdnStart, srvShtdnFinish time.Time
 
-	inst, err = test_helpers.StartTarantool(shtdnSrvOpts)
+	inst, err := test_helpers.StartTarantool(shtdnSrvOpts)
 	require.Nil(t, err)
 	defer test_helpers.StopTarantoolWithCleanup(inst)
 
@@ -492,7 +479,7 @@ func TestGracefulShutdownCloseConcurrent(t *testing.T) {
 			sret = cerr
 		}
 		srvStop.Done()
-	}(&inst)
+	}(inst)
 
 	srvStop.Wait()
 	require.Nil(t, sret, "No errors on server SIGTERM")
@@ -515,11 +502,9 @@ func TestGracefulShutdownCloseConcurrent(t *testing.T) {
 func TestGracefulShutdownConcurrent(t *testing.T) {
 	test_helpers.SkipIfWatchersUnsupported(t)
 
-	var inst test_helpers.TarantoolInstance
-	var err error
 	var srvShtdnStart, srvShtdnFinish time.Time
 
-	inst, err = test_helpers.StartTarantool(shtdnSrvOpts)
+	inst, err := test_helpers.StartTarantool(shtdnSrvOpts)
 	require.Nil(t, err)
 	defer test_helpers.StopTarantoolWithCleanup(inst)
 
@@ -582,7 +567,7 @@ func TestGracefulShutdownConcurrent(t *testing.T) {
 			sret = cerr
 		}
 		srvStop.Done()
-	}(&inst)
+	}(inst)
 
 	srvStop.Wait()
 	require.Nil(t, sret, "No errors on server SIGTERM")

tarantool_test.go

@@ -3609,7 +3609,7 @@ func TestConnection_NewWatcher_reconnect(t *testing.T) {
 	<-events
 
 	test_helpers.StopTarantool(inst)
-	if err := test_helpers.RestartTarantool(&inst); err != nil {
+	if err := test_helpers.RestartTarantool(inst); err != nil {
 		t.Fatalf("Unable to restart Tarantool: %s", err)
 	}
 
@@ -3902,7 +3902,7 @@ func TestConnection_named_index_after_reconnect(t *testing.T) {
 		t.Fatalf("An error expected.")
 	}
 
-	if err := test_helpers.RestartTarantool(&inst); err != nil {
+	if err := test_helpers.RestartTarantool(inst); err != nil {
 		t.Fatalf("Unable to restart Tarantool: %s", err)
 	}
 

test_helpers/main.go

@@ -21,8 +21,6 @@ import (
 	"path/filepath"
 	"regexp"
 	"strconv"
-	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/tarantool/go-tarantool/v2"
@@ -77,25 +75,10 @@ type StartOpts struct {
 	Dialer tarantool.Dialer
 }
 
-// errorWrap should be used with a pointer - as `optional` type.
-type errorWrap struct {
-	err error
-}
-
-type statusInstance struct {
-	// status if nil, then process is not done yet.
-	status *errorWrap
-	// isStopping is a flag that terminate was internally initiated.
-	isStopping atomic.Bool
-	// waitMutex is used to prevent several invokes of the "Wait"
-	// for the same process.
-	// https://github.com/golang/go/issues/28461
-	waitMutex sync.RWMutex
-}
-
-// statusInstanceWrap used to avoid capturing content in defer function.
-type statusInstanceWrap struct {
-	impl *statusInstance
+type state struct {
+	done   chan struct{}
+	ret    error
+	stoped bool
 }
 
 // TarantoolInstance is a data for instance graceful shutdown and cleanup.
@@ -109,64 +92,75 @@ type TarantoolInstance struct {
 	// Dialer to check that connection established.
 	Dialer tarantool.Dialer
 
-	st *statusInstanceWrap
-}
-
-func newTarantoolInstance() TarantoolInstance {
-	return TarantoolInstance{st: &statusInstanceWrap{&statusInstance{}}}
+	st chan state
 }
 
 func (t *TarantoolInstance) IsExit() bool {
-	succeeded := t.st.impl.waitMutex.TryRLock()
-	if !succeeded {
-		// Due to mutex locked by goroutine, it means that process running.
+	st := <-t.st
+	t.st<- st
+
+	select {
+	case <-st.done:
+	default:
 		return false
 	}
-	defer t.st.impl.waitMutex.RUnlock()
-	return t.st.impl.status != nil
+
+	return st.ret != nil
 }
 
 func (t *TarantoolInstance) result() error {
-	succeeded := t.st.impl.waitMutex.TryRLock()
-	if !succeeded {
+	st := <-t.st
+	t.st<- st
+
+	select {
+	case <-st.done:
+	default:
 		return nil
 	}
-	defer t.st.impl.waitMutex.RUnlock()
-	return t.st.impl.status.err
+
+	return st.ret
 }
 
 func (t *TarantoolInstance) checkDone() {
-	if t.st == nil || t.st.impl == nil {
-		panic("TarantoolInstance is not initialized properly.")
-	}
-
 	go func() {
-		t.st.impl.waitMutex.Lock()
-		defer t.st.impl.waitMutex.Unlock()
-		t.st.impl.status = &errorWrap{t.Cmd.Wait()}
-		if !t.st.impl.isStopping.Load() {
+		ret := t.Cmd.Wait()
+
+		st := <-t.st
+
+		st.ret = ret
+		close(st.done)
+
+		t.st<- st
+
+		if !st.stoped {
 			log.Printf("Tarantool %q was unexpected terminated: %v", t.Opts.Listen, t.result())
 		}
 	}()
 }
 
 func (t *TarantoolInstance) Wait() error {
-	if t.st == nil {
-		panic("TarantoolInstance is not initialized")
-	}
-	t.st.impl.waitMutex.RLock()
-	defer t.st.impl.waitMutex.RUnlock()
-	// Note: don't call `result()` here to avoid double locking.
-	return t.st.impl.status.err
+	st := <-t.st
+	t.st<- st
+
+	<-st.done
+
+	st = <-t.st
+	t.st<- st
+
+	return st.ret
 }
 
 func (t *TarantoolInstance) Stop() error {
 	if t == nil {
 		log.Print("ASSERT: no Tarantool instance")
 		return nil
 	}
+
+	st := <-t.st
+	st.stoped = true
+	t.st<- st
+
 	log.Printf("Stopping Tarantool instance %q", t.Opts.Listen)
-	t.st.impl.isStopping.Store(true)
 	if t.IsExit() {
 		log.Printf("Already stopped instance %q with result: %v", t.Opts.Listen, t.result())
 		return nil
@@ -295,9 +289,10 @@ func IsTarantoolEE() (bool, error) {
 func RestartTarantool(inst *TarantoolInstance) error {
 	log.Printf("Restarting Tarantool instance %q", inst.Opts.Listen)
 	startedInst, err := StartTarantool(inst.Opts)
+
 	inst.Cmd.Process = startedInst.Cmd.Process
-	// We can't change pointer to status instance, cause it could captured by `defer`.
-	inst.st.impl = startedInst.st.impl
+	inst.st = startedInst.st
+
 	return err
 }
 
@@ -342,11 +337,17 @@ func prepareDir(workDir string) (string, error) {
 // StartTarantool starts a tarantool instance for tests
 // with specifies parameters (refer to StartOpts).
 // Process must be stopped with StopTarantool.
-func StartTarantool(startOpts StartOpts) (TarantoolInstance, error) {
+func StartTarantool(startOpts StartOpts) (*TarantoolInstance, error) {
 	// Prepare tarantool command.
-	inst := newTarantoolInstance()
-	var err error
+	inst := &TarantoolInstance{
+		st: make(chan state, 1),
+	}
+	init := state{
+		done: make(chan struct{}),
+	}
+	inst.st <- init
 
+	var err error
 	inst.Dialer = startOpts.Dialer
 	startOpts.WorkDir, err = prepareDir(startOpts.WorkDir)
 	if err != nil {
@@ -426,13 +427,13 @@ func StartTarantool(startOpts StartOpts) (TarantoolInstance, error) {
 
 	if inst.IsExit() && inst.result() != nil {
 		StopTarantool(inst)
-		return TarantoolInstance{}, fmt.Errorf("unexpected terminated Tarantool %q: %w",
+		return &TarantoolInstance{}, fmt.Errorf("unexpected terminated Tarantool %q: %w",
 			inst.Opts.Listen, inst.result())
 	}
 
 	if err != nil {
 		StopTarantool(inst)
-		return TarantoolInstance{}, fmt.Errorf("failed to connect Tarantool %q: %w",
+		return &TarantoolInstance{}, fmt.Errorf("failed to connect Tarantool %q: %w",
 			inst.Opts.Listen, err)
 	}
 
@@ -442,7 +443,7 @@ func StartTarantool(startOpts StartOpts) (TarantoolInstance, error) {
 // StopTarantool stops a tarantool instance started
 // with StartTarantool. Waits until any resources
 // associated with the process is released. If something went wrong, fails.
-func StopTarantool(inst TarantoolInstance) {
+func StopTarantool(inst *TarantoolInstance) {
 	err := inst.Stop()
 	if err != nil {
 		log.Fatal(err)
@@ -453,7 +454,7 @@ func StopTarantool(inst TarantoolInstance) {
 // with StartTarantool. Waits until any resources
 // associated with the process is released.
 // Cleans work directory after stop. If something went wrong, fails.
-func StopTarantoolWithCleanup(inst TarantoolInstance) {
+func StopTarantoolWithCleanup(inst *TarantoolInstance) {
 	StopTarantool(inst)
 
 	if inst.Opts.WorkDir != "" {

test_helpers/pool_helper.go

@@ -227,8 +227,8 @@ func SetClusterRO(dialers []tarantool.Dialer, connOpts tarantool.Opts,
 	return nil
 }
 
-func StartTarantoolInstances(instsOpts []StartOpts) ([]TarantoolInstance, error) {
-	instances := make([]TarantoolInstance, 0, len(instsOpts))
+func StartTarantoolInstances(instsOpts []StartOpts) ([]*TarantoolInstance, error) {
+	instances := make([]*TarantoolInstance, 0, len(instsOpts))
 
 	for _, opts := range instsOpts {
 		instance, err := StartTarantool(opts)
@@ -243,7 +243,7 @@ func StartTarantoolInstances(instsOpts []StartOpts) ([]TarantoolInstance, error)
 	return instances, nil
 }
 
-func StopTarantoolInstances(instances []TarantoolInstance) {
+func StopTarantoolInstances(instances []*TarantoolInstance) {
 	for _, instance := range instances {
 		StopTarantoolWithCleanup(instance)
 	}