I hope you SAST to enlighten your development
If you don't use Static Analysis Tools (SAST) then you're missing out on a whole load of awesomeness which is generally free!
SAST Tools Rock
If you don't use Static Analysis Tools (SAST) then you're missing out on a whole load of awesomeness which is generally free!
Some come as standard with Visual Studio but there are some great others such as SonarLint and Code Cracker which can heklp find code smells, memory leaks, security issues and more.
Learning More....
I would also so add that no matter how long you have been programmimng, you will learn something from the errors and warnings they raise.
CA2000: Dispose objects before losing scope
I received this warning when I was doing some work with Sysytem.Timers in a Windows Service that used TopShelf which is also awesome. I've always avoided timers since my VB6 days when they were considered heavy, clunky and caused all sorts of problems.
Timers aren't so bad nowadays as they're just a wrapper for threads but they still need to be disposed correctly like all objects that implement IDisposable.
This is some demo code that shows the problem:
public class WindowsService : IDisposable
{
public Timer myTimer { get; set; }
public WindowsService()
{
myTimer = new System.Timers.Timer
{
Interval = 40000,
Enabled = true,
AutoReset = false
};
myTimer.Elapsed += MyTimer_Elapsed;
}
private void MyTimer_Elapsed(object sender, System.Timers.ElapsedEventArgs e)
{
//do some stuff
//the timer has AutoReset = false so we need to explicitly restart it
myTimer.Start();
}
public void Stop()
{
if (myTimer != null) myTimer.Stop();
}
public void Start()
{
if (myTimer != null) myTimer.Start();
}
#region IDisposable Support
private bool disposedValue = false;
protected virtual void Dispose(bool disposing)
{
if (!disposedValue)
{
if (disposing)
{
myTimer?.Dispose();
}
disposedValue = true;
}
}
public void Dispose()
{
Dispose(true);
}
#endregion
}
Now this looks fairly simple. There's a timer that gets initialised in the constructor, it has an event that is caught and it is disposed when the class gets disposed.
What's the problem?
Well, running Visual Studio SAST tools in VS2015
shows all sorts of useful warnings although some are less useful such as issues with naming convensions.
One warning that is showing for this code is this:
CA2000 In method 'WindowsService.WindowsService()', object 'new Timer()' is not disposed along all exception paths. Call System.IDisposable.Dispose on object 'new Timer()' before all references to it are out of scope.
The critical bit here is the message object 'new Timer()' is not disposed along all exception paths.
What happens when there is an exception during object initialisation? You guessed it, it does not get correctly disposed leading to a memory leak. This is because during initialisation a Timer is created then assigned to our variable. If an exception occurs during initialisation it causes one of these objects to remain leading to the memory leak.
Normally with a disposable object, you'll be creating it for one time use such as a Connection or some kind of Stream so you probably won't come across this issue, but with Timers they have a long life so need to be created with care.
Therefore just be careful using object initialisers on Disposabale objects and handle their disposal if an exception occurs. The following code makes the CA2000 warning disappear;
try
{
myTimer = new Timer();
myTimer.Interval = 40000;
myTimer.Enabled = true;
myTimer.AutoReset = false;
myTimer.Elapsed += MyTimer_Elapsed;
}
catch (Exception)
{
//do something
throw;
}
finally
{
myTimer?.Dispose();
}
Conclusions
SAST is awesome but also frustrating. Having analysers that check your code all the time can make Visual Studio slow. Running anaylsis during compilation can make it slow. Your best bet is to run it manually every so often, or even better build it into your build process so it is automated using TFS build controllers or in your CI process.