search

natefinch / lumberjack

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

fixed goroutine leak #57

Open jaczhao opened 6 years ago

jaczhao commented 6 years ago

I consider the Close() method is finalize call, so I make close(chan) call. the relate issue #56

natefinch commented 6 years ago

Sorry. I'll check it out tomorrow.

On Sun, Nov 19, 2017, 10:46 PM jack zhao notifications@github.com wrote:

@jaczhao commented on this pull request.

In lumberjack.go https://github.com/natefinch/lumberjack/pull/57#discussion_r151897771:

@@ -112,6 +112,7 @@ type Logger struct { mu sync.Mutex

millCh chan bool

  • wg sync.WaitGroup

@natefinch https://github.com/natefinch could you reply this?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/natefinch/lumberjack/pull/57#discussion_r151897771, or mute the thread https://github.com/notifications/unsubscribe-auth/ADCcyMy1Q27Qsngybx28Rg1TMxHjxTj0ks5s4PX_gaJpZM4QQnbH .

brunoamancio commented 6 years ago

@natefinch any news?

natefinch commented 6 years ago

I'll have to refresh myself on this. I think it's probably fine, but I want to make sure. Most people just start one logger and let.it run for the lifetime of the process. But if we can make it clean up better, that's definitely a good thing.

natefinch commented 6 years ago

This still needs a couple updates as specified above. I'll do the work myself if @jaczhao isn't interested anymore (not anyone's fault, it's been forever).

nmiyake commented 5 years ago

@natefinch or @jaczhao any plans on picking this up? I'm running into this now as well. If I don't hear an ACK, will post a new PR that picks up where this left off (will be based on this one and implement the feedback)

nmiyake commented 5 years ago

Opened https://github.com/natefinch/lumberjack/pull/80 as a PR that implements the requested changes to this PR and also adds a test.

howbazaar commented 4 years ago

I also opened #100 to address this issue. My PR also fixes the test synchronisation and determinism.

CaledoniaProject commented 2 years ago

Interesting, such a big and obvious bug, and this is still not merged after 3 years.

natefinch commented 2 years ago

It's not a big bug, since you don't normally restart your logging infrastructure multiple times. But you're right that it should get merged.

CaledoniaProject commented 2 years ago

It's a big bug to me. I have a program with multiple goroutines that each of them writes to a different log file.

Onine0811 commented 1 year ago

so cool, the bug has not been fixed yet.

gregtwallace commented 1 year ago

Onine0811

Unfortunately not it seems.

chensmiles commented 8 months ago

@natefinch any new plans about this bug？ there has already 3 PRs , will anyone get merged？

CaledoniaProject commented 8 months ago

We should create a new repo and dispose this one.