add retries to deleting locked pidfiles

watchdog/watchdog.go

@@ -60,39 +60,17 @@ func Watch(programs []Program, restartDelay time.Duration, log Log) error {
 	return nil
 }
 
-// terminate will send a kill signal to the program by finding its pid from the
-// path to its executable. If the program has specified a pidfile, this function
-// will open up that file and see if the pid in there matches the pid that was just
-// terminated. If so, this function will delete the file. It is much better for
-// programs to manage their own pidfiles, but kill signals cannot always be caught
-// and handled gracefully (i.e. in windows).
-func (p Program) terminate(log Log) {
-	log.Infof("Terminating %s", p.Path)
-	thisProcess := os.Getpid()
-	matcher := process.NewMatcher(p.Path, process.PathEqual, log)
-	matcher.ExceptPID(thisProcess)
-	// this logic also exists in the updater, so if you want to change it, look there too.
-	terminatedPids := process.TerminateAll(matcher, time.Second, log)
-	if len(terminatedPids) == 0 {
-		// nothing terminated probably because nothing was running
-		return
-	}
-	// if there was a pidfile, it might not have been cleaned up, so let's take a look
-	if p.PidFile == "" {
-		// nothing more to do
-		return
-	}
-	time.Sleep(500 * time.Millisecond) // give the program a chance to clean itself up
-	pidFromFile, err := ioutil.ReadFile(p.PidFile)
+func pidFileIsDeletable(terminatedPids []int, path string, log Log) (deletable bool) {
+	pidFromFile, err := ioutil.ReadFile(path)
 	if os.IsNotExist(err) {
 		// pidfile was successfully cleaned up by the terminated process
 		// or was never created
-		return
+		return false
 	}
 	lockedPid, err := strconv.Atoi(string(pidFromFile))
 	if err != nil {
-		log.Infof("error reading pid from file after terminating program %s: %s", p.Path, err.Error())
-		return
+		log.Infof("error reading pid from file after terminating program %s: %s", path, err.Error())
+		return false
 	}
 	var terminatedPidIsStillLockedByFile bool
 	for _, termPid := range terminatedPids {
@@ -101,15 +79,58 @@ func (p Program) terminate(log Log) {
 			break
 		}
 	}
-	if !terminatedPidIsStillLockedByFile {
-		// the program updated its own pidfile
+	return terminatedPidIsStillLockedByFile
+}
+
+func (p Program) deletePidFileIfTerminated(terminatedPids []int, log Log) {
+	if p.PidFile == "" {
+		// there is no pidfile on this program
 		return
 	}
-	if err := os.Remove(p.PidFile); err != nil {
-		log.Infof("error deleting pidfile %s: %s", p.PidFile, err.Error())
+	var err error
+	ticker := time.NewTicker(300 * time.Millisecond)
+	timeout := time.NewTimer(3 * time.Second)
+	for {
+		select {
+		case <-ticker.C:
+			if !pidFileIsDeletable(terminatedPids, p.PidFile, log) {
+				return
+			}
+			err = os.Remove(p.PidFile)
+			if err != nil {
+				log.Infof("unable to delete a pidfile %s: %s", p.PidFile, err.Error())
+				continue
+			}
+			log.Infof("successfully deleted a pidfile %s", p.PidFile)
+			return
+		case <-timeout.C:
+			// time out
+			log.Infof("timed out trying to delete a pidfile %s", p.PidFile)
+			return
+		}
+	}
+}
+
+// terminate will send a kill signal to the program by finding its pid from the
+// path to its executable. If the program has specified a pidfile, this function
+// will open up that file and see if the pid in there matches the pid that was just
+// terminated. If so, this function will delete the file. It is much better for
+// programs to manage their own pidfiles, but kill signals cannot always be caught
+// and handled gracefully (i.e. in windows).
+func (p Program) terminate(log Log) {
+	log.Infof("Terminating %s", p.Path)
+	thisProcess := os.Getpid()
+	matcher := process.NewMatcher(p.Path, process.PathEqual, log)
+	matcher.ExceptPID(thisProcess)
+	// this logic also exists in the updater, so if you want to change it, look there too.
+	terminatedPids := process.TerminateAll(matcher, time.Second, log)
+	if len(terminatedPids) == 0 {
+		// nothing terminated probably because nothing was running
 		return
 	}
-	log.Infof("Successfully deleted a pid file at %s", p.PidFile)
+	// if there was a pidfile, it might not have been cleaned up, so let's take a look
+	p.deletePidFileIfTerminated(terminatedPids, log)
+	return
 }
 
 func terminateExisting(programs []Program, log Log) {