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

Performance Counter Instances Never Removed #146

Closed
mackenzieajudd opened this issue Sep 2, 2016 · 5 comments
Closed

Performance Counter Instances Never Removed #146

mackenzieajudd opened this issue Sep 2, 2016 · 5 comments

Comments

@mackenzieajudd
Copy link
Contributor

mackenzieajudd commented Sep 2, 2016

We ran into a "Custom counters file view is out of memory" exception in our application running in IIS which brought this issue to light.

We have performance counters enabled as instructed here: https://github.com/castleproject/Windsor/blob/master/docs/performance-counters.md

I found that the TrackedComponentsPerformanceCounterWrapper class never removes the PerformanceCounter instance that it wraps on disposal. If performance counters are enabled for an application which is restarted often the available performance counter memory can be exhausted because a new performance counter, with a unique name, is created every time on startup.

It would be nice if TrackedComponentsPerformanceCounterWrapper removed the wrapped performance counter instance on disposal.

To reproduce:

  1. Create a console app with performance counters enabled (https://github.com/castleproject/Windsor/blob/master/docs/performance-counters.md)
  2. Start and stop the app multiple times
  3. Open Performance Monitor and look at the Castle Windsor category. You should see a performance counter for each time you started the app.
@jonorossi
Copy link
Member

@mackenzieajudd I'm don't think the intention of the Windsor performance counters was to be used in production always enabled, there shouldn't be any harm though other than this defect obviously. However, won't an application restart cause IIS to tear down the application pool process without really "shutting down" applications resulting in the same outcome?

The code currently calls PerformanceCounter.Dispose during clean up, a quick skim of the MSDN docs seems to indicate this is all that is required. What do you mean by remove the performance counter?

Any chance you could provide a pull request with the fix?

@mackenzieajudd
Copy link
Contributor Author

@jonorossi thanks for the quick response!

You may be right about the IIS teardown I'm not sure.

From what I can see PerformanceCounter.Dispose releases the resources it is using but won't remove a performance counter from the machine. When you create a new instance of PerformanceCounter it will either hook into an existing performance counter if the Category and Name already exist or create a new one if not.

In our case Windsor creates a new performance counter each time because its name is unique (Based on process name and id) to account for multiple instances being tracked at the same time. Because it's not re-using an existing performance counter instance I think the instance should be removed when Windsor is done with it so they don't pile up.

I'll put something together and hopefully have a pull request this week!

I've attached a picture to help illustrate what I'm talking about. The performance counters shown in it have all been created by Windsor but where not removed when the container was disposed and the application was closed.

multipleperfcounters

@jonorossi
Copy link
Member

@mackenzieajudd sounds logical to remove the instance on container shutdown, they are obviously pretty useless once the container has shutdown. I'd leave the category around though.

Have you looked at setting PerformanceCounter.InstanceLifetime = PerformanceCounterInstanceLifetime.Process, so the counter instances will automatically be removed on process shutdown. After a quick skim it sounds like performance counter instances are built on Windows handles so even if your IIS worker process gets killed the counter instance will get removed because there is no reference to it anymore.

@jonorossi
Copy link
Member

You'd also have to change the constructor usage of PerformanceCounter as most initialize the counter instance which means you can't set the lifetime. Looks like the only one you can use is the no arguments constructor:

http://referencesource.microsoft.com/#System/services/monitoring/system/diagnosticts/PerformanceCounter.cs,7cd5e59dff25918d,references

mackenzieajudd added a commit to mackenzieajudd/Windsor that referenced this issue Sep 21, 2016
@mackenzieajudd
Copy link
Contributor Author

Thanks @jonorossi! I opened a pull request fixing this.

jonorossi added a commit that referenced this issue Sep 22, 2016
Fix for issue #146: Performance Counter Instances Never Removed
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

No branches or pull requests

2 participants