natefinch / lumberjack

lumberjack is a log rolling package for Go
MIT License
4.8k stars 591 forks source link

goroutine leak #56

Open jaczhao opened 6 years ago

jaczhao commented 6 years ago

millrun goroutine leak.

I use zap to wrap lumberjack, my code is following

w := zapcore.AddSync(&lumberjack.Logger{
  Filename:   "/var/log/myapp/foo.log",
  MaxSize:    500, // megabytes
  MaxBackups: 3,
  MaxAge:     28, // days
})
core := zapcore.NewCore(
  zapcore.NewJSONEncoder(zap.NewProductionEncoderConfig()),
  w,
  zap.InfoLevel,
)
logger := zap.New(core)

when I do testing, I got the following error:

Too many goroutines running after all test(s).
1 instances of:

**/vendor/gopkg.in/natefinch/lumberjack%2ev2.(*Logger).millRun(...)
**/vendor/gopkg.in/natefinch/lumberjack.v2/lumberjack.go:379 +0x58
created by **/vendor/gopkg.in/natefinch/lumberjack%2ev2.(*Logger).mill.func1
***/vendor/gopkg.in/natefinch/lumberjack.v2/lumberjack.go:390 +0x7e
ERROR: *manager.**TestSuite is failed by leak goroutine check.

I tried to used Close() method, but it still failed.

I read source code and fount that nowhere to close the logger.millCh, so this goroutine will live forever.

natefinch commented 6 years ago

You're right that we do have a Close method, and that close method could stop the mill goroutine fairly trivially. I'll get that fix out sometime soon, or you're welcome to send in a PR :)

jaczhao commented 6 years ago

one PR, please review it, Thanks

openmohan commented 6 years ago

Experiencing the same issue . Thanks @jaczhao for the solution.