Restart gracefully with in-process restart

[tpng]

This patch allows Caddy to restart in-process gracefully without the -restart=inproc flag, replacing the current default Restart on POSIX system.

[mholt]

Any race condition here? (Edit: On second thought, probably not, since Start() gets called in same goroutine as the signal trap. I'll double-check anyway.)

[mholt]

No race condition.

Follow-up though, could we revise this line to remove the lint notice:

caddy\restartinproc.go:9:16: should omit type map[string]*os.File from declaration of var restartFds; it will be inferred from the right-hand side

So, change to:

var restartFds = make(map[string]*os.File)

[mholt]

Woah. I have yet to try this, but if this works and is free of races, let's drop the "new process" restart altogether and just use this (and drop the -restart option)!

[mholt]

This works wonderfully. The race detector does not detect any races.

@tpng I really love this. Going to merge in after I or others have a chance to review more closely!

[mholt]

Wait, does this also work on Windows?

[tpng]

Actually I don't know how to trigger a Caddy restart on Windows since the USR1 is only available on POSIX system. If there are other conditions to trigger a restart on Windows I can test it to see if it works.

[tpng]

It seems Go doesn't implement the FileListener function on WIndows, so it will not work on Windows, will need to use the original method on Windows.

[mholt]

For triggering a restart on Windows, I was just thinking of a test that calls the Restart() function.

But since FileListener isn't implemented on Windows (I knew that once, but since forgot), we will still have the regular "Windows" restart that we have now. But all other restarts can be switched to this. 👍

[tpng]

OK, so I need to update the pull request to do the following?
1. Move the original restart logic back to restart_windows.go
2. Update restart.go to use the new logic
3. Remove restartinproc.go
4. Remove -restart flag

[captncraig]

What if the "windows restart" was the current improc one and we note in
docs that Windows has occasional (very brief) downtime when it renews certs?

On Friday, May 6, 2016, Benny Ng notifications@github.com wrote:

OK, so I need to update the pull request to do the following?

1. Move the original restart logic back to restart_windows.go
2. Update restart.go to use the new logic.
3. Remove restartinproc.go
4. Remove -restart flag

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#807 (comment)

[tpng]

Yes, Windows will use the current inproc one.

[mholt]

We should have this godoc comment updated too, removing the part about piping in as part of a restart.

[mholt]

Benny, this is looking beautiful. I think I'm gonna cry. 😂

[mholt]

Great work @tpng, thank you!!