diff --git a/caddyhttp/httpserver/roller.go b/caddyhttp/httpserver/roller.go index b8264636..0660de65 100644 --- a/caddyhttp/httpserver/roller.go +++ b/caddyhttp/httpserver/roller.go @@ -2,6 +2,7 @@ package httpserver import ( "io" + "path/filepath" "strconv" "github.com/mholt/caddy" @@ -19,14 +20,30 @@ type LogRoller struct { } // GetLogWriter returns an io.Writer that writes to a rolling logger. +// This should be called only from the main goroutine (like during +// server setup) because this method is not thread-safe; it is careful +// to create only one log writer per log file, even if the log file +// is shared by different sites or middlewares. This ensures that +// rolling is synchronized, since a process (or multiple processes) +// should not create more than one roller on the same file at the +// same time. See issue #1363. func (l LogRoller) GetLogWriter() io.Writer { - return &lumberjack.Logger{ - Filename: l.Filename, - MaxSize: l.MaxSize, - MaxAge: l.MaxAge, - MaxBackups: l.MaxBackups, - LocalTime: l.LocalTime, + absPath, err := filepath.Abs(l.Filename) + if err != nil { + absPath = l.Filename // oh well, hopefully they're consistent in how they specify the filename } + lj, has := lumberjacks[absPath] + if !has { + lj = &lumberjack.Logger{ + Filename: l.Filename, + MaxSize: l.MaxSize, + MaxAge: l.MaxAge, + MaxBackups: l.MaxBackups, + LocalTime: l.LocalTime, + } + lumberjacks[absPath] = lj + } + return lj } // ParseRoller parses roller contents out of c. @@ -62,3 +79,7 @@ func ParseRoller(c *caddy.Controller) (*LogRoller, error) { LocalTime: true, }, nil } + +// lumberjacks maps log filenames to the logger +// that is being used to keep them rolled/maintained. +var lumberjacks = make(map[string]*lumberjack.Logger)