From d5a912a1b19edcc40bcba277569e04717855ee75 Mon Sep 17 00:00:00 2001 From: kewegner <47402596+kewegner@users.noreply.github.com> Date: Thu, 14 Nov 2019 10:51:45 -0600 Subject: [PATCH] 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 --- main.go | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 89 insertions(+), 7 deletions(-) diff --git a/main.go b/main.go index 5cb975e..df87187 100644 --- a/main.go +++ b/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,7 +540,7 @@ 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) @@ -534,11 +548,12 @@ func runCommands(commandString string, theProcessType ProcessType, killServer bo } 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 + } + + } + +}