4

There are certain static methods such as Process.Start() and File.Create() that construct and return IDisposable instances that are often discarded. It's so normal to use these methods as though they return void that if you aren't paying attention you might miss the fact that they even have return values at all.

I understand that it's good practice to always dispose of IDisposable instances. Does this apply to unused return values? Instead of writing Process.Start(processPath); should you always write Process.Start(processPath).Dispose();?

I'd think this would be an obvious yes but I have to second guess myself because I only ever see it done without the Dispose(). Does C# perhaps have some automatic way of handling those situations?

Kyle Delaney
  • 11,616
  • 6
  • 39
  • 66
  • 1
    All usual considerations apply here as well, those cases are not exceptions. And it should not be "so normal to use these methods as though they return void". – Evk Apr 23 '18 at 14:06
  • 2
    Yes, there is an automatic way to handle it: if a class encapsulates unmanaged resources itself (like, say, a process handle) it'll have a finalizer that takes care of releasing those resources if and when the garbage collector gets around to it. Obviously that's not as good as disposing of things as soon as you're done with them. The reason you often see people get sloppy is because the code is a one-off, or it will never create more than one process or file before exiting. In this case the fact that you're not disposing ASAP goes unnoticed. – Jeroen Mostert Apr 23 '18 at 14:07
  • 1
    Whether the object is returned by a call you make or initialized by code you write is irrelevant. What matters is the consequences of not cleaning up resources. – P.Brian.Mackey Apr 23 '18 at 14:07
  • 4
    You shouldn't use `Process.Start(processPath).Dispose()` but `using(var p = Process.Start(processPath)){}`. It's possible that `Start` throws an exception, in that case the latter approach will ensure that it's disposed anyway. – Tim Schmelter Apr 23 '18 at 14:11
  • 1
    @TimSchmelter `Process.Start(..).Dispose()` is no different from `using` in regards to leaked handles and such. If Start throws exception - there is nothing to dispose from outside of Start function. – Evk Apr 23 '18 at 14:30
  • 1
    @TimSchmelter: if `Process.Start` throws an exception, the method will never return a result, handle or no. The only thing the `using` add is protection for the (very small) window where the method has returned the result and then something like a `ThreadAbortException` occurs, but then, that could also happen just before the `Start` returns, so that really doesn't add much. When you're worrying about stuff like that, things like constrained execution regions come into play. As an application programmer you usually don't worry about that; if those things happen it's up to the finalizer. – Jeroen Mostert Apr 23 '18 at 14:34
  • 1
    The Process class is lazy, it only allocates something worth disposable if you actually use the returned object to, say, use its WaitForExit() method. That is an obscure factoid that is not particularly obvious, other than from the way the api is designed. If they make it easy to not dispose an object then that usually means you don't have to. The opposite example is the Bitmap class, very important to dispose, you always have to use its constructor. Nothing to fret about, it isn't wrong to dispose even when it is not necessary. – Hans Passant Apr 23 '18 at 14:38
  • 1
    @HansPassant I don't think it's true. It allocates process handle right away, and it's the main thing to release. Or is it? – Evk Apr 23 '18 at 15:49

2 Answers2

6

In most of the cases, prefered way of dealing with IDisposable instances is by using using statement:

using (var file = File.Create())
{
   //use file instance
}

This syntactic sugar will make sure that file instance is explicitly disposed after you exit using statement block. Under the hood, it actually wraps your code into try/finally block.

Darjan Bogdan
  • 3,780
  • 1
  • 22
  • 31
  • 2
    But don't do it with `HttpClient` and create an application-wide static instance instead :D – ProgrammingLlama Apr 23 '18 at 14:15
  • @john yes you are right, however it's another topic due to some underlying architectural flaws inside HttpClient itself :) – Darjan Bogdan Apr 23 '18 at 14:16
  • is this the same question ? - https://stackoverflow.com/questions/3039196/method-returns-an-idisposable-should-i-dispose-of-the-result-even-if-its-not?rq=1 – Mauricio Gracia Gutierrez Apr 23 '18 at 14:20
  • 3
    This answer doesn't quite reflect my question. You're talking about a situation where the return value is assigned to a variable and then used. I'm talking about a situation where the return value is unused. In that case, the using block would be empty. That feels too awkward for me to call it syntactic sugar. – Kyle Delaney Apr 23 '18 at 15:07
  • this answers HOW to handle `IDisposable` instances, but does not address the original question – Mauricio Gracia Gutierrez Apr 23 '18 at 15:13
  • 1
    @KyleDelaney I agree, I had to be more precise, in the end preferred way would mean that no matter of certain use case, created instance should be disposed :) – Darjan Bogdan Apr 23 '18 at 15:52
  • 1
    I understand that `using` has some subtle advantages over `Dispose()`, but the empty block looks like a mistake so I may still go with `Dispose()` for improved readability. – Kyle Delaney Apr 23 '18 at 16:45
5

.NET has concept of Finalizer and many BCL types which implement IDisposable, also implement Finalizer. You can check MSDN Dispose Pattern article and this CodeProject post for more details. So if you won't dispose some object manually, it will likely be disposed later anyway. The question is if later (and other potential issues) is OK for you, or you need it to be disposed explicitly and deterministically at some specific point.

Downsides of skipping Dispose method call and relying on Finalizer are described in this thread and include:

  1. Performance issues
  2. Issues with locked resources which are in use longer than you expect
  3. Crash risk even if you have exception handling policy

In general I would strongly recommend always calling the Dispose method explicitly to avoid quirky problems.

But you should dispose only those objects which you own (you created and know when they are not used anymore). I've faced situations when I use some IDisposable resource, get another IDisposable as property of it and obviously I shouldn't dispose a property of an object before I stop using the referencing object - it will do it on it's own when disposed.

Sasha
  • 8,537
  • 4
  • 49
  • 76