Signal handler improved (#51)

* temporary key change

* reverted key

* change image name and remove script file

* image name changes CMD added

* new branch

* PR comments and logging

Author: kewegner

main.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"os/signal"
 	"regexp"
 	"strconv"
 	"strings"
@@ -308,7 +309,7 @@ func setupEnvironmentVars() error {
 	return err
 }
 
-func killProcess(theProcessType ProcessType) error {
+func killProcess(theProcessType ProcessType, checkAttempts int) error {
 	var processPid int
 	var err error
 	if theProcessType == server {
@@ -319,10 +320,24 @@ func killProcess(theProcessType ProcessType) error {
 	ControllerDebug.log("Attempting to kill pid: ", processPid)
 
 	if processPid != 0 {
+		// Check to see if the process is still alive to avoid unncessary kill steps
+		if cmps.processes[theProcessType].Signal(syscall.Signal(0)) != nil {
+			ControllerDebug.log("No such process for pid:  ", processPid)
+			err = nil
+		} else {
 
-		ControllerDebug.log("Killing pid:  ", processPid)
-		err = syscall.Kill(-processPid, syscall.SIGINT)
-
+			ControllerDebug.log("Killing pid:  ", processPid)
+			err = syscall.Kill(-processPid, syscall.SIGINT)
+			// If checkAttempts speified check and wait to make sure process was killed.
+			for i := 0; i < checkAttempts; i++ {
+				ControllerDebug.log("Process check ", theProcessType, i)
+				if cmps.processes[theProcessType].Signal(syscall.Signal(0)) != nil {
+					break //process is dead
+				} else {
+					time.Sleep(2 * time.Second)
+				}
+			}
+		}
 		cmps.processes[theProcessType] = nil
 		cmps.pids[theProcessType] = 0
 		if err != nil {
@@ -445,7 +460,6 @@ func runWatcher(fileChangeCommand string, dirs []string, killServer bool) error
 
 			case err := <-w.Error:
 				ControllerWarning.log("An error occured in the file watcher ", err)
-
 			case <-w.Closed:
 				ControllerDebug.log("The file watcher is now closed")
 				return
@@ -526,19 +540,20 @@ func runCommands(commandString string, theProcessType ProcessType, killServer bo
 		// This is a watcher
 		if killServer {
 			ControllerDebug.log("APPSODY_RUN/DEBUG/TEST_ON_KILL is true, attempting to kill the corresponding process.")
-			err = killProcess(server)
+			err = killProcess(server, 0)
 			if err != nil {
 				// do nothing we continue after kill errors
 				ControllerWarning.log("The attempt to kill the process received an error ", err)
 			}
 		}
 		ControllerDebug.log("Killing the APPSODY_RUN/DEBUG/TEST_ON_CHANGE process.")
 
-		err = killProcess(fileWatcher)
+		err = killProcess(fileWatcher, 0)
 		if err != nil {
 			// do nothing we continue after kill errors
 			ControllerWarning.log("Killing the the APPSODY_RUN/DEBUG/TEST_ON_CHANGE process received error ", err)
 		}
+		go reapChildProcesses(2)
 
 		commandToUse := commandString
 		processTypeToUse := fileWatcher
@@ -695,6 +710,33 @@ func main() {
 
 		go runCommands(startCommand, server, false, false)
 	}
+
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, os.Interrupt, syscall.SIGTERM, syscall.SIGINT)
+	go func() {
+		<-c
+		cmps.mu.Lock()
+		defer cmps.mu.Unlock()
+		ControllerDebug.log("Inside signal handler for controller")
+		ControllerDebug.log("Killing the ON_CHANGE process")
+		// In practice either the fileWatcher or server process will be alive, not both
+		err := killProcess(fileWatcher, 2) // we give 2 *2 seconds to kill the filewatcher/ON_CHANGE process
+		if err != nil {
+			ControllerError.log("Received error during signal handler killing ON_CHANGE process", err)
+		}
+		ControllerDebug.log("Killing the server process")
+		err = killProcess(server, 2) // we give 2 *2 seconds to kill the server process
+		if err != nil {
+			ControllerError.log("Received error during signal handler killing the RUN/TEST/DEBUG process", err)
+		}
+		// 5 * 1 second waiting for reaping of child processes
+		// This call is allowed to complete by the fact that the docker stop allows 10 seconds for processeing
+		// prior to the sig kill
+		go reapChildProcesses(5) //run separately to make sure that we don't block
+
+		ControllerDebug.log("Done processing controller signal handler.")
+	}()
+
 	if fileChangeCommand != "" && !disableWatcher {
 
 		err = runWatcher(fileChangeCommand, dirs, stopWatchServerOnChange)
@@ -709,3 +751,43 @@ func main() {
 	}
 
 }
+
+func reapChildProcesses(maxLimit int) {
+	countLimit := 0
+
+	for {
+
+		var wstatus syscall.WaitStatus
+		//WNOHANG means return if there are no child processes to wait for
+		//This command will wait for processes that hae been reassigned
+		// to pid 1 after the server or fileWatcher/ON_CHANGE process is terminated
+		pid, err := syscall.Wait4(-1, &wstatus, syscall.WNOHANG, nil)
+		ControllerDebug.log("Reaper pid/err is: ", pid, err)
+		// If it is 0 that means no process was waiting atm, we will sleep and give a little more time
+		if pid == 0 && countLimit < maxLimit && err == nil {
+			ControllerDebug.log("Reaper sleeping 1 second: ", pid)
+			time.Sleep(1 * time.Second)
+			countLimit++
+		}
+
+		if syscall.EINTR == err {
+			// A Signal Interupt occured and we should stop processing
+			ControllerDebug.log("Signal Interrupt: ", err)
+			break
+		}
+		//This value means no child processes left waiting.
+		if syscall.ECHILD == err {
+			ControllerDebug.log("No more child processes: ", err)
+			break
+		}
+
+		ControllerDebug.log("Max limit count: ", countLimit)
+
+		if countLimit >= maxLimit {
+			ControllerDebug.log("Max limit reached: ", maxLimit)
+			break
+		}
+
+	}
+
+}