From 99d47050e97a8ccac2aad3bda46be46d4fec85ed Mon Sep 17 00:00:00 2001 From: Matt Holt Date: Thu, 2 Mar 2023 21:00:18 -0700 Subject: [PATCH] core: Eliminate unnecessary shutdown delay on Unix (#5413) * core: Eliminate unnecessary shutdown delay on Unix Fix #5393, alternate to #5405 * Comments, cleanup, adjust logs * Fix build constraint --- go.mod | 2 +- listen.go | 5 +---- listen_unix.go | 36 +++++++++++++++++++++++++++++++----- modules/caddyhttp/app.go | 6 +++--- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 4a5dc864..1b6dbb92 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/caddyserver/caddy/v2 -go 1.18 +go 1.19 require ( github.com/BurntSushi/toml v1.2.1 diff --git a/listen.go b/listen.go index 1ca16874..e0d67c6a 100644 --- a/listen.go +++ b/listen.go @@ -12,10 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// TODO: Go 1.19 introduced the "unix" build tag. We have to support Go 1.18 until Go 1.20 is released. -// When Go 1.19 is our minimum, change this build tag to simply "!unix". -// (see similar change needed in listen_unix.go) -//go:build !(aix || android || darwin || dragonfly || freebsd || hurd || illumos || ios || linux || netbsd || openbsd || solaris) +//go:build !unix package caddy diff --git a/listen_unix.go b/listen_unix.go index 7ea67452..50d9a66d 100644 --- a/listen_unix.go +++ b/listen_unix.go @@ -12,10 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -// TODO: Go 1.19 introduced the "unix" build tag. We have to support Go 1.18 until Go 1.20 is released. -// When Go 1.19 is our minimum, remove this build tag, since "_unix" in the filename will do this. -// (see also change needed in listen.go) -//go:build aix || android || darwin || dragonfly || freebsd || hurd || illumos || ios || linux || netbsd || openbsd || solaris +// Even though the filename ends in _unix.go, we still have to specify the +// build constraint here, because the filename convention only works for +// literal GOOS values, and "unix" is a shortcut unique to build tags. +//go:build unix package caddy @@ -98,7 +98,19 @@ func listenTCPOrUnix(ctx context.Context, lnKey string, network, address string, } return reusePort(network, address, c) } - return config.Listen(ctx, network, address) + + // even though SO_REUSEPORT lets us bind the socket multiple times, + // we still put it in the listenerPool so we can count how many + // configs are using this socket; necessary to ensure we can know + // whether to enforce shutdown delays, for example (see #5393). + ln, err := config.Listen(ctx, network, address) + if err == nil { + listenerPool.LoadOrStore(lnKey, nil) + } + + // lightly wrap the listener so that when it is closed, + // we can decrement the usage pool counter + return deleteListener{ln, lnKey}, err } // reusePort sets SO_REUSEPORT. Ineffective for unix sockets. @@ -116,3 +128,17 @@ func reusePort(network, address string, conn syscall.RawConn) error { } }) } + +// deleteListener is a type that simply deletes itself +// from the listenerPool when it closes. It is used +// solely for the purpose of reference counting (i.e. +// counting how many configs are using a given socket). +type deleteListener struct { + net.Listener + lnKey string +} + +func (dl deleteListener) Close() error { + _, _ = listenerPool.Delete(dl.lnKey) + return dl.Listener.Close() +} diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index 36a4011b..670185a1 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -517,7 +517,7 @@ func (app *App) Stop() error { // honor scheduled/delayed shutdown time if delay { - app.logger.Debug("shutdown scheduled", + app.logger.Info("shutdown scheduled", zap.Duration("delay_duration", time.Duration(app.ShutdownDelay)), zap.Time("time", scheduledTime)) time.Sleep(time.Duration(app.ShutdownDelay)) @@ -528,9 +528,9 @@ func (app *App) Stop() error { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, time.Duration(app.GracePeriod)) defer cancel() - app.logger.Debug("servers shutting down; grace period initiated", zap.Duration("duration", time.Duration(app.GracePeriod))) + app.logger.Info("servers shutting down; grace period initiated", zap.Duration("duration", time.Duration(app.GracePeriod))) } else { - app.logger.Debug("servers shutting down with eternal grace period") + app.logger.Info("servers shutting down with eternal grace period") } // goroutines aren't guaranteed to be scheduled right away,