-
Notifications
You must be signed in to change notification settings - Fork 47
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
Possible thread leak when ILogger instances are disposed. #63
Comments
Hey @CoderNumber1, I think is it a good enhancement. |
@merbla Would you mind if I forked the repo and took a shot an resolving the issue myself? I would enjoy the opportunity to submit a pull request. |
@CoderNumber1 absolutely go for it!! 👍 |
@merbla Sorry for the radio silence, I've had to put this down in favor of some other projects recently so I haven't been able to devote much time to it. I've been doing some testing today though that looks promising. I'm running both the original and periodic batching versions side by side in separate executables trying to re-create a thread leak in each. The former is up over 3000 threads so far while the updated version has been holding steady at 3 for the last 30 minutes or so. I'll try to submit a pull request for this in the next few days. |
👍 great work! |
I have a web application which spins up and disposes of logs as needed while it's running. After adding in the Splunk sink I noticed the application running slower with a much higher CPU utilization. It looked like the application's thread count would just grow constantly. Looking at the
EventCollectorSink
constructor I notice it callsRepeatAction.OnInterval(TimeSpan pollInterval, Action action, CancellationToken token)
but doesn't cancel the returned task on dispose. My guess is then each time anILogger
is spun up a new task/thread is created and it continues to attempt to run after theILogger
is disposed until the web app recycles. I wonder if the returned task could be held in a private field by the sink and cancelled on dispose or maybe even let that sink inherit from the Periodic Batch Sink and let that deal with batching log messages.https://github.com/serilog/serilog-sinks-splunk/blob/092d929ea92624f98970844e46e91e7878db54b4/src/Serilog.Sinks.Splunk/Sinks/Splunk/EventCollectorSink.cs#L193
https://github.com/serilog/serilog-sinks-splunk/blob/092d929ea92624f98970844e46e91e7878db54b4/src/Serilog.Sinks.Splunk/Sinks/Splunk/RepeatAction.cs#L26
The text was updated successfully, but these errors were encountered: