Merge pull request #1404 from mholt/combinedrollers

Create only one log roller per file across whole process (fixes #1363)

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)