Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

goroutine leak #56

Open
jaczhao opened this issue Nov 2, 2017 · 3 comments · May be fixed by #80
Open

goroutine leak #56

jaczhao opened this issue Nov 2, 2017 · 3 comments · May be fixed by #80

Comments

@jaczhao
Copy link

jaczhao commented Nov 2, 2017

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.

@jaczhao jaczhao changed the title I use goroutine leak Nov 2, 2017
@natefinch
Copy link
Owner

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
Copy link
Author

jaczhao commented Nov 3, 2017

one PR, please review it, Thanks

@openmohan
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants