Skip to content

Commit d56b9c8

Browse files
merge master
2 parents 301391f + 7a18aae commit d56b9c8

10 files changed

+272
-47
lines changed

Diff for: .travis.yml

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
language: go
2+
3+
go:
4+
- 1.0
5+
- 1.1
6+
- 1.2
7+
- 1.3
8+
- release
9+
- tip

Diff for: bench_test.go

+42
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,45 @@ func BenchmarkMultiLevelFilter(b *testing.B) {
8585
lg.Info("test message")
8686
}
8787
}
88+
89+
func BenchmarkDescendant1(b *testing.B) {
90+
lg := New()
91+
lg.SetHandler(DiscardHandler())
92+
lg = lg.New()
93+
for i := 0; i < b.N; i++ {
94+
lg.Info("test message")
95+
}
96+
}
97+
98+
func BenchmarkDescendant2(b *testing.B) {
99+
lg := New()
100+
lg.SetHandler(DiscardHandler())
101+
for i := 0; i < 2; i++ {
102+
lg = lg.New()
103+
}
104+
for i := 0; i < b.N; i++ {
105+
lg.Info("test message")
106+
}
107+
}
108+
109+
func BenchmarkDescendant4(b *testing.B) {
110+
lg := New()
111+
lg.SetHandler(DiscardHandler())
112+
for i := 0; i < 4; i++ {
113+
lg = lg.New()
114+
}
115+
for i := 0; i < b.N; i++ {
116+
lg.Info("test message")
117+
}
118+
}
119+
120+
func BenchmarkDescendant8(b *testing.B) {
121+
lg := New()
122+
lg.SetHandler(DiscardHandler())
123+
for i := 0; i < 8; i++ {
124+
lg = lg.New()
125+
}
126+
for i := 0; i < b.N; i++ {
127+
lg.Info("test message")
128+
}
129+
}

Diff for: ext/handler.go

+10-11
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
package ext
22

33
import (
4-
log "gopkg.in/inconshreveable/log15.v2"
54
"sync"
5+
"sync/atomic"
6+
"unsafe"
7+
8+
log "gopkg.in/inconshreveable/log15.v2"
69
)
710

811
// EscalateErrHandler wraps another handler and passes all records through
@@ -95,23 +98,19 @@ func (h *Speculative) Flush() {
9598
// used to implement the SetHandler method for the default
9699
// implementation of Logger.
97100
func HotSwapHandler(h log.Handler) *HotSwap {
98-
return &HotSwap{handler: h}
101+
hs := new(HotSwap)
102+
hs.Swap(h)
103+
return hs
99104
}
100105

101106
type HotSwap struct {
102-
mu sync.RWMutex
103-
handler log.Handler
107+
handler unsafe.Pointer
104108
}
105109

106110
func (h *HotSwap) Log(r *log.Record) error {
107-
defer h.mu.RUnlock()
108-
h.mu.RLock()
109-
err := h.handler.Log(r)
110-
return err
111+
return (*(*log.Handler)(atomic.LoadPointer(&h.handler))).Log(r)
111112
}
112113

113114
func (h *HotSwap) Swap(newHandler log.Handler) {
114-
h.mu.Lock()
115-
defer h.mu.Unlock()
116-
h.handler = newHandler
115+
atomic.StorePointer(&h.handler, unsafe.Pointer(&newHandler))
117116
}

Diff for: handler.go

+8-12
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"os"
99
"reflect"
1010
"sync"
11+
"sync/atomic"
12+
"unsafe"
1113

1214
"gopkg.in/inconshreveable/log15.v2/stack"
1315
)
@@ -123,7 +125,7 @@ func CallerStackHandler(format string, h Handler) Handler {
123125
return FuncHandler(func(r *Record) error {
124126
s := stack.Callers().
125127
TrimBelow(stack.Call(r.CallPC[0])).
126-
TrimAboveName("main.main")
128+
TrimRuntime()
127129
if len(s) > 0 {
128130
buf := &bytes.Buffer{}
129131
buf.WriteByte('[')
@@ -277,24 +279,18 @@ func BufferedHandler(bufSize int, h Handler) Handler {
277279
return ChannelHandler(recs)
278280
}
279281

280-
// swapHandler wraps another handler that may swapped out
282+
// swapHandler wraps another handler that may be swapped out
281283
// dynamically at runtime in a thread-safe fashion.
282284
type swapHandler struct {
283-
mu sync.RWMutex
284-
handler Handler
285+
handler unsafe.Pointer
285286
}
286287

287288
func (h *swapHandler) Log(r *Record) error {
288-
defer h.mu.RUnlock()
289-
h.mu.RLock()
290-
err := h.handler.Log(r)
291-
return err
289+
return (*(*Handler)(atomic.LoadPointer(&h.handler))).Log(r)
292290
}
293291

294292
func (h *swapHandler) Swap(newHandler Handler) {
295-
h.mu.Lock()
296-
defer h.mu.Unlock()
297-
h.handler = newHandler
293+
atomic.StorePointer(&h.handler, unsafe.Pointer(&newHandler))
298294
}
299295

300296
// LazyHandler writes all values to the wrapped handler after evaluating
@@ -316,7 +312,7 @@ func LazyHandler(h Handler) Handler {
316312
} else {
317313
if cs, ok := v.(stack.Trace); ok {
318314
v = cs.TrimBelow(stack.Call(r.CallPC[0])).
319-
TrimAboveName("main.main")
315+
TrimRuntime()
320316
}
321317
r.Ctx[i] = v
322318
}

Diff for: log15_test.go

+89
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net"
1010
"regexp"
1111
"runtime"
12+
"sync"
1213
"testing"
1314
"time"
1415
)
@@ -446,3 +447,91 @@ func TestCallerFuncHandler(t *testing.T) {
446447
t.Fatalf("Wrong context value, got %s expected string matching %s", s, regex)
447448
}
448449
}
450+
451+
// https://github.com/inconshreveable/log15/issues/27
452+
func TestCallerStackHandler(t *testing.T) {
453+
t.Parallel()
454+
455+
l := New()
456+
h, r := testHandler()
457+
l.SetHandler(CallerStackHandler("%#v", h))
458+
459+
lines := []int{}
460+
461+
func() {
462+
l.Info("baz")
463+
_, _, line, _ := runtime.Caller(0)
464+
lines = append(lines, line-1)
465+
}()
466+
_, file, line, _ := runtime.Caller(0)
467+
lines = append(lines, line-1)
468+
469+
if len(r.Ctx) != 2 {
470+
t.Fatalf("Expected stack in record context. Got length %d, expected %d", len(r.Ctx), 2)
471+
}
472+
473+
const key = "stack"
474+
475+
if r.Ctx[0] != key {
476+
t.Fatalf("Wrong context key, got %s expected %s", r.Ctx[0], key)
477+
}
478+
479+
s, ok := r.Ctx[1].(string)
480+
if !ok {
481+
t.Fatalf("Wrong context value type, got %T expected string", r.Ctx[1])
482+
}
483+
484+
exp := "["
485+
for i, line := range lines {
486+
if i > 0 {
487+
exp += " "
488+
}
489+
exp += fmt.Sprint(file, ":", line)
490+
}
491+
exp += "]"
492+
493+
if s != exp {
494+
t.Fatalf("Wrong context value, got %s expected string matching %s", s, exp)
495+
}
496+
}
497+
498+
// tests that when logging concurrently to the same logger
499+
// from multiple goroutines that the calls are handled independently
500+
// this test tries to trigger a previous bug where concurrent calls could
501+
// corrupt each other's context values
502+
//
503+
// this test runs N concurrent goroutines each logging a fixed number of
504+
// records and a handler that buckets them based on the index passed in the context.
505+
// if the logger is not concurrent-safe then the values in the buckets will not all be the same
506+
//
507+
// https://github.com/inconshreveable/log15/pull/30
508+
func TestConcurrent(t *testing.T) {
509+
root := New()
510+
// this was the first value that triggered
511+
// go to allocate extra capacity in the logger's context slice which
512+
// was necessary to trigger the bug
513+
const ctxLen = 34
514+
l := root.New(make([]interface{}, ctxLen)...)
515+
const goroutines = 8
516+
var res [goroutines]int
517+
l.SetHandler(SyncHandler(FuncHandler(func(r *Record) error {
518+
res[r.Ctx[ctxLen+1].(int)]++
519+
return nil
520+
})))
521+
var wg sync.WaitGroup
522+
wg.Add(goroutines)
523+
for i := 0; i < goroutines; i++ {
524+
go func(idx int) {
525+
defer wg.Done()
526+
for j := 0; j < 10000; j++ {
527+
l.Info("test message", "goroutine_idx", idx)
528+
}
529+
}(i)
530+
}
531+
wg.Wait()
532+
for _, val := range res[:] {
533+
if val != 10000 {
534+
t.Fatalf("Wrong number of messages for context: %+v", res)
535+
}
536+
}
537+
}

Diff for: logger.go

+12-2
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func (l *logger) write(msg string, lvl Lvl, ctx []interface{}) {
100100
Time: time.Now(),
101101
Lvl: lvl,
102102
Msg: msg,
103-
Ctx: append(l.ctx, normalize(ctx)...),
103+
Ctx: newContext(l.ctx, ctx),
104104
KeyNames: RecordKeyNames{
105105
Time: timeKey,
106106
Msg: msgKey,
@@ -112,7 +112,17 @@ func (l *logger) write(msg string, lvl Lvl, ctx []interface{}) {
112112
}
113113

114114
func (l *logger) New(ctx ...interface{}) Logger {
115-
return &logger{append(l.ctx, normalize(ctx)...), &swapHandler{handler: l.h}}
115+
child := &logger{newContext(l.ctx, ctx), new(swapHandler)}
116+
child.SetHandler(l.h)
117+
return child
118+
}
119+
120+
func newContext(prefix []interface{}, suffix []interface{}) []interface{} {
121+
normalizedSuffix := normalize(suffix)
122+
newCtx := make([]interface{}, len(prefix)+len(normalizedSuffix))
123+
n := copy(newCtx, prefix)
124+
copy(newCtx[n:], suffix)
125+
return newCtx
116126
}
117127

118128
func (l *logger) Debug(msg string, ctx ...interface{}) {

Diff for: root.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package log15
33
import (
44
"os"
55

6+
"github.com/mattn/go-colorable"
67
"gopkg.in/inconshreveable/log15.v2/term"
78
)
89

@@ -14,14 +15,15 @@ var (
1415

1516
func init() {
1617
if term.IsTty(os.Stdout.Fd()) {
17-
StdoutHandler = StreamHandler(os.Stdout, TerminalFormat())
18+
StdoutHandler = StreamHandler(colorable.NewColorableStdout(), TerminalFormat())
1819
}
1920

2021
if term.IsTty(os.Stderr.Fd()) {
21-
StderrHandler = StreamHandler(os.Stderr, TerminalFormat())
22+
StderrHandler = StreamHandler(colorable.NewColorableStderr(), TerminalFormat())
2223
}
2324

24-
root = &logger{[]interface{}{}, &swapHandler{handler: StdoutHandler}}
25+
root = &logger{[]interface{}{}, new(swapHandler)}
26+
root.SetHandler(StdoutHandler)
2527
}
2628

2729
// New returns a new logger with the given context.

0 commit comments

Comments
 (0)