Commit 45a0e4cf authored by Matthew Holt's avatar Matthew Holt

log: Fix race when stopping server

High improbability of being an actual problem. Logs are safe for
concurrent use, but os.Files are apparently not... Fixes #1371.
parent e14a62f1
...@@ -6,6 +6,7 @@ import ( ...@@ -6,6 +6,7 @@ import (
"log" "log"
"net/http" "net/http"
"os" "os"
"sync"
"github.com/mholt/caddy" "github.com/mholt/caddy"
"github.com/mholt/caddy/caddyhttp/httpserver" "github.com/mholt/caddy/caddyhttp/httpserver"
...@@ -54,7 +55,9 @@ func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { ...@@ -54,7 +55,9 @@ func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
// Write log entries // Write log entries
for _, e := range rule.Entries { for _, e := range rule.Entries {
e.fileMu.RLock()
e.Log.Println(rep.Replace(e.Format)) e.Log.Println(rep.Replace(e.Format))
e.fileMu.RUnlock()
} }
return status, err return status, err
...@@ -70,6 +73,7 @@ type Entry struct { ...@@ -70,6 +73,7 @@ type Entry struct {
Log *log.Logger Log *log.Logger
Roller *httpserver.LogRoller Roller *httpserver.LogRoller
file *os.File // if logging to a file that needs to be closed file *os.File // if logging to a file that needs to be closed
fileMu *sync.RWMutex // files can't be safely read/written in one goroutine and closed in another (issue #1371)
} }
// Rule configures the logging middleware. // Rule configures the logging middleware.
......
...@@ -7,6 +7,7 @@ import ( ...@@ -7,6 +7,7 @@ import (
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"strings" "strings"
"sync"
"testing" "testing"
"github.com/mholt/caddy/caddyhttp/httpserver" "github.com/mholt/caddy/caddyhttp/httpserver"
...@@ -29,6 +30,7 @@ func TestLoggedStatus(t *testing.T) { ...@@ -29,6 +30,7 @@ func TestLoggedStatus(t *testing.T) {
Entries: []*Entry{{ Entries: []*Entry{{
Format: DefaultLogFormat + " {testval}", Format: DefaultLogFormat + " {testval}",
Log: log.New(&f, "", 0), Log: log.New(&f, "", 0),
fileMu: new(sync.RWMutex),
}}, }},
} }
...@@ -72,6 +74,7 @@ func TestLogRequestBody(t *testing.T) { ...@@ -72,6 +74,7 @@ func TestLogRequestBody(t *testing.T) {
Entries: []*Entry{{ Entries: []*Entry{{
Format: "{request_body}", Format: "{request_body}",
Log: log.New(&got, "", 0), Log: log.New(&got, "", 0),
fileMu: new(sync.RWMutex),
}}, }},
}}, }},
Next: httpserver.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) { Next: httpserver.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
...@@ -131,10 +134,12 @@ func TestMultiEntries(t *testing.T) { ...@@ -131,10 +134,12 @@ func TestMultiEntries(t *testing.T) {
{ {
Format: "foo {request_body}", Format: "foo {request_body}",
Log: log.New(&got1, "", 0), Log: log.New(&got1, "", 0),
fileMu: new(sync.RWMutex),
}, },
{ {
Format: "{method} {request_body}", Format: "{method} {request_body}",
Log: log.New(&got2, "", 0), Log: log.New(&got2, "", 0),
fileMu: new(sync.RWMutex),
}, },
}, },
}}, }},
......
...@@ -5,6 +5,7 @@ import ( ...@@ -5,6 +5,7 @@ import (
"log" "log"
"os" "os"
"path/filepath" "path/filepath"
"sync"
"github.com/hashicorp/go-syslog" "github.com/hashicorp/go-syslog"
"github.com/mholt/caddy" "github.com/mholt/caddy"
...@@ -65,7 +66,9 @@ func setup(c *caddy.Controller) error { ...@@ -65,7 +66,9 @@ func setup(c *caddy.Controller) error {
for _, rule := range rules { for _, rule := range rules {
for _, entry := range rule.Entries { for _, entry := range rule.Entries {
if entry.file != nil { if entry.file != nil {
entry.fileMu.Lock()
entry.file.Close() entry.file.Close()
entry.fileMu.Unlock()
} }
} }
} }
...@@ -111,6 +114,7 @@ func logParse(c *caddy.Controller) ([]*Rule, error) { ...@@ -111,6 +114,7 @@ func logParse(c *caddy.Controller) ([]*Rule, error) {
OutputFile: DefaultLogFilename, OutputFile: DefaultLogFilename,
Format: DefaultLogFormat, Format: DefaultLogFormat,
Roller: logRoller, Roller: logRoller,
fileMu: new(sync.RWMutex),
}) })
} else if len(args) == 1 { } else if len(args) == 1 {
// Only an output file specified // Only an output file specified
...@@ -118,6 +122,7 @@ func logParse(c *caddy.Controller) ([]*Rule, error) { ...@@ -118,6 +122,7 @@ func logParse(c *caddy.Controller) ([]*Rule, error) {
OutputFile: args[0], OutputFile: args[0],
Format: DefaultLogFormat, Format: DefaultLogFormat,
Roller: logRoller, Roller: logRoller,
fileMu: new(sync.RWMutex),
}) })
} else { } else {
// Path scope, output file, and maybe a format specified // Path scope, output file, and maybe a format specified
...@@ -139,6 +144,7 @@ func logParse(c *caddy.Controller) ([]*Rule, error) { ...@@ -139,6 +144,7 @@ func logParse(c *caddy.Controller) ([]*Rule, error) {
OutputFile: args[1], OutputFile: args[1],
Format: format, Format: format,
Roller: logRoller, Roller: logRoller,
fileMu: new(sync.RWMutex),
}) })
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment