tt: fix the race condition between start and stop

This patch fixes the signals handling strategy. Now there is only
one handling loop for all signals. Added two fields for controlling
a state of the instance being watched and a mutex for synchronizing
the goroutines changing them.

Author: vr009

cli/running/running.go

@@ -6,7 +6,6 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
-	"os/signal"
 	"path"
 	"path/filepath"
 	"strings"
@@ -20,7 +19,6 @@ import (
 	"github.com/tarantool/tt/cli/process_utils"
 	"github.com/tarantool/tt/cli/ttlog"
 	"github.com/tarantool/tt/cli/util"
-	"golang.org/x/net/context"
 	"gopkg.in/yaml.v2"
 )
 
@@ -516,32 +514,20 @@ func FillCtx(cliOpts *config.CliOpts, cmdCtx *cmdcontext.CmdCtx,
 
 // Start an Instance.
 func Start(cmdCtx *cmdcontext.CmdCtx, run *InstanceCtx) error {
-	// Register a context, related with a signal handler that
-	// removes a pid file if interrupt signals were sent before starting
-	// an instance and watchdog signal handlers.
-	sigCtx, stop := signal.NotifyContext(context.Background(),
-		syscall.SIGINT, syscall.SIGTERM)
+	logger := createLogger(run)
+	provider := providerImpl{cmdCtx: cmdCtx, instanceCtx: run}
+	wd := NewWatchdog(run.Restartable, 5*time.Second, logger, &provider)
+	wd.StartSignalHandling()
+
 	if err := process_utils.CreatePIDFile(run.PIDFile); err != nil {
 		return err
 	}
 
 	defer func() {
 		cleanup(run)
-		stop()
 	}()
 
-	logger := createLogger(run)
-	provider := providerImpl{cmdCtx: cmdCtx, instanceCtx: run}
-	wd := NewWatchdog(run.Restartable, 5*time.Second, logger, &provider)
-
-	// The Done() state of the context means that the process received
-	// an interrupt signal and there is a need to clean up the resources.
-	select {
-	case <-sigCtx.Done():
-		return nil
-	default:
-		wd.Start()
-	}
+	wd.Start()
 	return nil
 }
 

cli/running/watchdog.go

@@ -35,10 +35,12 @@ type Watchdog struct {
 	// restartTimeout describes the timeout between
 	// restarting the Instance.
 	restartTimeout time.Duration
-	// stopped indicates whether Watchdog was stopped.
-	stopped bool
-	// stopMutex used to avoid a race condition under stopped field.
-	stopMutex sync.Mutex
+	// watchdogStateMutex used to avoid a race condition under started and shouldStop fields.
+	watchdogStateMutex sync.Mutex
+	// started indicates whether Watchdog has started an Instance.
+	started bool
+	// shouldStop indicates whether Watchdog should stop the Instance.
+	shouldStop bool
 	// done channel used to inform the signal handle goroutine
 	// about termination of the Instance.
 	done chan bool
@@ -55,7 +57,7 @@ func NewWatchdog(restartable bool, restartTimeout time.Duration, logger *ttlog.L
 		provider: provider}
 
 	wd.done = make(chan bool, 1)
-	wd.stopped = false
+	wd.shouldStop = false
 
 	return &wd
 }
@@ -72,23 +74,32 @@ func (wd *Watchdog) Start() {
 			break
 		}
 		wd.logger = wd.Instance.logger
-		// Start the Instance and forwarding signals (except  SIGINT and SIGTERM)
-		wd.startSignalHandling()
-		wd.stopMutex.Lock()
-		if !wd.stopped {
+
+		wd.watchdogStateMutex.Lock()
+		if !wd.shouldStop {
+			// Start the Instance and forwarding signals (except  SIGINT and SIGTERM)
 			if err := wd.Instance.Start(); err != nil {
 				wd.logger.Printf(`Watchdog(ERROR): "%v".`, err)
-				wd.stopMutex.Unlock()
+				wd.watchdogStateMutex.Unlock()
 				break
 			}
+			wd.started = true
+		} else {
+			wd.logger.Printf(`Watchdog(ERROR): terminated before start.`)
+			wd.watchdogStateMutex.Unlock()
+			return
 		}
-		wd.stopMutex.Unlock()
+		wd.watchdogStateMutex.Unlock()
 
 		// Wait while the Instance will be terminated.
 		if err := wd.Instance.Wait(); err != nil {
 			wd.logger.Printf(`Watchdog(WARN): "%v".`, err)
 		}
 
+		wd.watchdogStateMutex.Lock()
+		wd.started = false
+		wd.watchdogStateMutex.Unlock()
+
 		// Set Instance process completion indication.
 		wd.done <- true
 		// Wait for the signal processing goroutine to complete.
@@ -100,7 +111,7 @@ func (wd *Watchdog) Start() {
 			wd.logger.Println("Watchdog(ERROR): can't check if the instance is restartable.")
 			break
 		}
-		if wd.stopped || !restartable {
+		if wd.shouldStop || !restartable {
 			wd.logger.Println("Watchdog(INFO): the Instance has shutdown.")
 			break
 		}
@@ -112,15 +123,18 @@ func (wd *Watchdog) Start() {
 			wd.logger = logger
 		}
 		time.Sleep(wd.restartTimeout)
+
+		wd.shouldStop = false
+		// Before the restart of an instance start a new signal handling loop.
+		wd.StartSignalHandling()
 	}
 }
 
-// startSignalHandling starts signal handling in a separate goroutine.
-func (wd *Watchdog) startSignalHandling() {
+// StartSignalHandling starts signal handling in a separate goroutine.
+func (wd *Watchdog) StartSignalHandling() {
 	sigChan := make(chan os.Signal, 1)
-	// Reset unregisters all previous handlers for interrupt signals.
-	signal.Reset(syscall.SIGINT,
-		syscall.SIGTERM, syscall.SIGHUP)
+	// Reset the signal mask before starting of the new loop.
+	signal.Reset()
 	signal.Notify(sigChan)
 
 	// Set barrier to synchronize with the main loop when the Instance stops.
@@ -136,20 +150,25 @@ func (wd *Watchdog) startSignalHandling() {
 			case sig := <-sigChan:
 				switch sig {
 				case syscall.SIGINT, syscall.SIGTERM:
-					wd.stopMutex.Lock()
-					wd.Instance.Stop(30 * time.Second)
+					wd.watchdogStateMutex.Lock()
+					if wd.started {
+						wd.Instance.Stop(30 * time.Second)
+					}
 					// If we receive one of the "stop" signals, the
 					// program should be terminated.
-					wd.stopped = true
-					wd.stopMutex.Unlock()
+					wd.shouldStop = true
+					wd.watchdogStateMutex.Unlock()
 				case syscall.SIGHUP:
 					// Rotate the log files.
 					wd.logger.Rotate()
 				default:
-					wd.Instance.SendSignal(sig)
+					wd.watchdogStateMutex.Lock()
+					if wd.started {
+						wd.Instance.SendSignal(sig)
+					}
+					wd.watchdogStateMutex.Unlock()
 				}
 			case _ = <-wd.done:
-				signal.Reset()
 				return
 			}
 		}