diff --git a/caddy/caddy.go b/caddy/caddy.go index a8e12bc34..20099aa79 100644 --- a/caddy/caddy.go +++ b/caddy/caddy.go @@ -59,7 +59,7 @@ var ( // errIncompleteRestart occurs if this process is a fork // of the parent but no Caddyfile was piped in - errIncompleteRestart = errors.New("cannot finish restart successfully") + errIncompleteRestart = errors.New("incomplete restart") // servers is a list of all the currently-listening servers servers []*server.Server @@ -103,15 +103,16 @@ const ( // In any case, an error is returned if Caddy could not be // started. func Start(cdyfile Input) (err error) { - // When we return, tell the parent whether we started - // successfully, and if so, write the pidfile (if enabled) + // If we return with no errors, we must do two things: tell the + // parent that we succeeded and write to the pidfile. defer func() { - success := err == nil - signalParent(success) - if success && PidFile != "" { - err := writePidFile() - if err != nil { - log.Printf("[ERROR] Could not write pidfile: %v", err) + if err == nil { + signalSuccessToParent() // TODO: Is doing this more than once per process a bad idea? Start could get called more than once in other apps. + if PidFile != "" { + err := writePidFile() + if err != nil { + log.Printf("[ERROR] Could not write pidfile: %v", err) + } } } }() diff --git a/caddy/helpers.go b/caddy/helpers.go index 01a35fc45..8d615055a 100644 --- a/caddy/helpers.go +++ b/caddy/helpers.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "io/ioutil" + "log" "os" "os/exec" "runtime" @@ -39,16 +40,16 @@ func checkFdlimit() { } } -// signalParent tells the parent our status using pipe at index 3. +// signalSuccessToParent tells the parent our status using pipe at index 3. // If this process is not a restart, this function does nothing. -// Calling this is vital so that the parent process can unblock and -// either continue running or kill itself. -func signalParent(success bool) { +// Calling this function once this process has successfully initialized +// is vital so that the parent process can unblock and kill itself. +func signalSuccessToParent() { if IsRestart() { - ppipe := os.NewFile(3, "") // parent is listening on pipe at index 3 - if success { - // Tell parent process that we're OK so it can quit now - ppipe.Write([]byte("success")) + ppipe := os.NewFile(3, "") // parent is reading from pipe at index 3 + _, err := ppipe.Write([]byte("success")) // we must send some bytes to the parent + if err != nil { + log.Printf("[ERROR] Communicating successful init to parent: %v", err) } ppipe.Close() } diff --git a/caddy/restart.go b/caddy/restart.go index d6acd0cf3..c41f27e40 100644 --- a/caddy/restart.go +++ b/caddy/restart.go @@ -8,7 +8,6 @@ import ( "log" "os" "os/exec" - "syscall" ) func init() { @@ -54,56 +53,56 @@ func Restart(newCaddyfile Input) error { } // Prepare a pipe that the child process will use to communicate - // its success or failure with us, the parent + // its success with us by sending > 0 bytes sigrpipe, sigwpipe, err := os.Pipe() if err != nil { return err } - // Pass along current environment and file descriptors to child. - // Ordering here is very important: stdin, stdout, stderr, sigpipe, - // and then the listener file descriptors (in order). - fds := []uintptr{rpipe.Fd(), os.Stdout.Fd(), os.Stderr.Fd(), sigwpipe.Fd()} + // Pass along relevant file descriptors to child process; ordering + // is very important since we rely on these being in certain positions. + extraFiles := []*os.File{sigwpipe} - // Now add file descriptors of the sockets + // Add file descriptors of all the sockets serversMu.Lock() for i, s := range servers { - fds = append(fds, s.ListenerFd()) + extraFiles = append(extraFiles, s.ListenerFd()) cdyfileGob.ListenerFds[s.Addr] = uintptr(4 + i) // 4 fds come before any of the listeners } serversMu.Unlock() - // We're gonna need the proper path to the executable - exepath, err := exec.LookPath(os.Args[0]) + // Set up the command + cmd := exec.Command(os.Args[0], os.Args[1:]...) + cmd.Stdin = rpipe // fd 0 + cmd.Stdout = os.Stdout // fd 1 + cmd.Stderr = os.Stderr // fd 2 + cmd.ExtraFiles = extraFiles + + // Spawn the child process + err = cmd.Start() if err != nil { return err } - // Fork the process with the current environment and file descriptors - execSpec := &syscall.ProcAttr{ - Env: os.Environ(), - Files: fds, - } - _, err = syscall.ForkExec(exepath, os.Args, execSpec) - if err != nil { - return err - } - - // Feed it the Caddyfile + // Feed Caddyfile to the child err = gob.NewEncoder(wpipe).Encode(cdyfileGob) if err != nil { return err } wpipe.Close() - // Wait for child process to signal success or fail - sigwpipe.Close() // close our copy of the write end of the pipe or we might be stuck - answer, err := ioutil.ReadAll(sigrpipe) - if err != nil || len(answer) == 0 { - log.Println("[ERROR] Restart: child failed to initialize; changes not applied") + // Determine whether child startup succeeded + sigwpipe.Close() // close our copy of the write end of the pipe -- TODO: why? + answer, readErr := ioutil.ReadAll(sigrpipe) + if answer == nil || len(answer) == 0 { + cmdErr := cmd.Wait() // get exit status + log.Printf("[ERROR] Restart: child failed to initialize (%v) - changes not applied", cmdErr) + if readErr != nil { + log.Printf("[ERROR] Restart: additionally, error communicating with child process: %v", readErr) + } return errIncompleteRestart } - // Child process is listening now; we can stop all our servers here. + // Looks like child was successful; we can exit gracefully. return Stop() } diff --git a/caddy/sigtrap_posix.go b/caddy/sigtrap_posix.go index e2f326b75..2d9898f42 100644 --- a/caddy/sigtrap_posix.go +++ b/caddy/sigtrap_posix.go @@ -19,6 +19,8 @@ func init() { for { <-reload + log.Println("[INFO] SIGUSR1: Reloading") + var updatedCaddyfile Input caddyfileMu.Lock() @@ -42,7 +44,7 @@ func init() { err := Restart(updatedCaddyfile) if err != nil { - log.Printf("[ERROR] SIGUSR1: Restart returned: %v", err) + log.Printf("[ERROR] SIGUSR1: %v", err) } } }() diff --git a/main.go b/main.go index a6c37ff68..1a468631c 100644 --- a/main.go +++ b/main.go @@ -30,7 +30,7 @@ const ( func init() { flag.BoolVar(&letsencrypt.Agreed, "agree", false, "Agree to Let's Encrypt Subscriber Agreement") - flag.StringVar(&letsencrypt.CAUrl, "ca", "https://acme-staging.api.letsencrypt.org", "Certificate authority ACME server") + flag.StringVar(&letsencrypt.CAUrl, "ca", "https://acme-staging.api.letsencrypt.org/directory", "Certificate authority ACME server") flag.StringVar(&conf, "conf", "", "Configuration file to use (default="+caddy.DefaultConfigFile+")") flag.StringVar(&cpu, "cpu", "100%", "CPU cap") flag.StringVar(&letsencrypt.DefaultEmail, "email", "", "Default Let's Encrypt account email address") @@ -62,7 +62,7 @@ func main() { default: file, err := os.OpenFile(logfile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644) if err != nil { - log.Fatalf("Error opening log file: %v", err) + log.Fatalf("Error opening process log file: %v", err) } log.SetOutput(file) } @@ -95,11 +95,7 @@ func main() { // Start your engines err = caddy.Start(caddyfile) if err != nil { - if caddy.IsRestart() { - log.Printf("[ERROR] Upon starting %s: %v", appName, err) - } else { - mustLogFatal(err) - } + mustLogFatal(err) } // Twiddle your thumbs @@ -107,10 +103,15 @@ func main() { } // mustLogFatal just wraps log.Fatal() in a way that ensures the -// output is always printed to stderr so the user can see it, -// even if the process log was not enabled. +// output is always printed to stderr so the user can see it +// if the user is still there, even if the process log was not +// enabled. If this process is a restart, however, and the user +// might not be there anymore, this just logs to the process log +// and exits. func mustLogFatal(args ...interface{}) { - log.SetOutput(os.Stderr) + if !caddy.IsRestart() { + log.SetOutput(os.Stderr) + } log.Fatal(args...) } diff --git a/server/server.go b/server/server.go index 9971bc955..4aaa1d965 100644 --- a/server/server.go +++ b/server/server.go @@ -280,14 +280,11 @@ func (s *Server) WaitUntilStarted() { } // ListenerFd gets the file descriptor of the listener. -func (s *Server) ListenerFd() uintptr { +func (s *Server) ListenerFd() *os.File { s.listenerMu.Lock() defer s.listenerMu.Unlock() - file, err := s.listener.File() - if err != nil { - return 0 - } - return file.Fd() + file, _ := s.listener.File() + return file } // ServeHTTP is the entry point for every request to the address that s