Method returns an IDisposable - Should I dispose of the result, even if it's not assigned to anything?

This seems like a fairly straightforward question, but I couldn't find this particular use-case after some searching around.

Suppose I have a simple method that, say, determines if a file is opened by some process. I can do this (not 100% correctly, but fairly well) with this:

public bool IsOpen(string fileName)
{
  try
  {
    File.Open(fileName, FileMode.Open, FileAccess.Read, FileShare.None);
  }
  catch
  {
    // if an exception is thrown, the file must be opened by some other process
    return true;
  } 
}

(obviously this isn't the best or even correct way to determine this - File.Open throws a number of different exceptions, all with different meanings, but it works for this example)

Now the File.Open call returns a Fi开发者_StackOverflow中文版leStream, and FileStream implements IDisposable. Normally we'd want to wrap the usage of any FileStream instantiations in a using block to make sure they're disposed of properly. But what happens in the case where we don't actually assign the return value to anything? Is it still necessary to dispose of the FileStream, like so:

try
{
  using (File.Open(fileName, FileMode.Open, FileAccess.Read, FileShare.None));
  { /* nop */ }
}
catch 
{
  return true;
}

Should I create a FileStream instance and dispose of that?

try
{
  using (FileStream fs = File.Open(fileName, FileMode.Open, FileAccess.Read, FileShare.None));
}
...

Or are these totally unnecessary? Can we simply call File.Open and not assign it to anything (first code example), and let the GC dispose of it right away?

Yes, you should definitely dispose of the FileStream. Otherwise the stream will remain open and the file won't be usable until a finalizer happens to clean it up.

The important thing here is ownership: for File.Open, the caller is assumed to "own" the stream returned to it - and if you own something which implements IDisposable, it's your responsibility to dispose of it.

Compare this with the situation of Image.FromStream: in that case, you pass in a stream and the Image then assumes that it owns that stream. You mustn't close the stream yourself, in that case - you have to dispose of the image when you're done, and it will dispose of the stream.

Calling a static method which returns something disposable almost always assumes that the caller takes ownership of the resource. Ditto constructors (which are effectively static methods.)

The irony in this case is that if you don't dispose of the stream returned by File.Open, you'll have found that the file is usable - at the same time as making it unusable until some indeterminate time.

If a method returns an IDisposable, I personally would always put it in a using block. Even if I don't assign the return value to anything.

Even if you don't assign it to a variable, the disposable object is still created. Dispose is not going to be called automatically. The only difference will be that the returned object will become immediately eligible for garbage collection, because there are no (strong) references to it.

The garbage collector does not call Dispose automatically when it reclaims an object. However, most IDisposable types provide a finalizer (which will be called just before the GC reclaims an object) that invokes Dispose as a fallback strategy (safety net) — study the IDisposable pattern to see how this is done:

~SomeClass          // <-- the finalizer method will usually call Dispose;
{                   //     but you have no control over when it will be called!
    Dispose(false);
}

Remember that you don't know when the garbage collector will run (because it's non-deterministic). Therefore, you also don't know when the finalizer method will be called. And because of that -- if you haven't called Dispose explicitly (either yourself, or with a using block) -- you don't know when it will be called by the finalizer.

That's the advantage of calling Dispose explicitly: You can free resources -- or at least allow the GC to free managed resources -- as soon as you're done with them, instead of holding on to resources until the finalizer gets called sometime in the future.

Yes, you don't want to leave the FileStream opened. For one, you won't even be able to open the file yourself after that. Calling Close() is good enough, but using using is probably the preferred pattern.

There's a much bigger problem with your code however. It cannot possibly work reliably on Windows. A typical scenario:

• The File.Open() call succeeds. You close it
• Your thread gets pre-empted by the Windows scheduler
• Another thread in another process gets a chance to run, it opens the file
• Your thread regains the CPU and continues after the File.Open() call
• You open the file, trusting that it will work since IsOpen() returned false.

Kaboom.

Never write code like this, failure is extremely hard to diagnose. Only ever open a file when you are ready to start reading or writing to it. And don't close it until you are done.

Extra bonus: it is now obvious that you want to use a using statement.

When you call any method that returns something, an instance of that something is being created. Just because you're not actually capturing it, doesn't make it any less "there". Therefore, in the case of an IDisposable object, the object is being created by the method you're calling in spite of the fact that you're doing nothing with it. So yes, you still need to dispose of it somehow. The first approach with the using statement seems like it should work.