Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Can I "inline" a variable if it's IDisposable?

Do I have to do this to ensure the MemoryStream is disposed of properly?

  using (MemoryStream stream = new MemoryStream(bytes))
  using (XmlReader reader = XmlReader.Create(stream))
  {
    return new XmlDocument().Load(reader);
  }

or is it OK to inline the MemoryStream so that it simply goes out of scope? Like this?

  using (XmlReader reader = XmlReader.Create(new MemoryStream(bytes)))
  {
    return new XmlDocument().Load(reader);
  }
like image 903
Colin Avatar asked Feb 11 '10 15:02

Colin


5 Answers

As a general rule, yes, you should write the code as in the first example.

There are some classes that take ownership of the object passed to it, so that when you dispose the outer object, it automatically disposes of the inner object for you, but that's the exception to the rule.

In any case, calling Dispose more than once is supposed to be safe. That is, objects should implement that so that it is safe, only doing the work the first time.

So as a general rule, go with the first syntax.

Now, for the specified example, it shouldn't really matter, as a MemoryStream isn't really holding on to any resources that needs to be disposed of, but there is a problem with that expectation too. If you know that a given version of an object doesn't use a resource, so it's safe to ignore the Dispose, then if that object in the future gains such a resource, you suddenly gain a leak.

Unless you're seeing some adverse effect with the given code, like adding too much overhead, then I would simply not worry about it.

like image 94
Lasse V. Karlsen Avatar answered Nov 02 '22 20:11

Lasse V. Karlsen


The XmlReader does not by default (but see Colin's and dh's suggestion) assume that it is the only one using a stream, so the first option is the only Dispose safe one.

like image 31
Pontus Gagge Avatar answered Nov 02 '22 19:11

Pontus Gagge


There is an option to use XmlReaderSettings and set CloseInput to true like this

var reader = XmlReader.Create(new MemoryStream(), new XmlReaderSettings {CloseInput = true});

Here: XmlReaderSettings.CloseInput Property

like image 26
dh. Avatar answered Nov 02 '22 19:11

dh.


This really depends on the Dispose() of XmlReader. It would take some work to figure out exactly what it does. I personally write code like the first sample. If you new something, then it is your responsibility to dispose it. You shouldn't expect others to take care of it for you (although they may).

like image 3
Bryan Avatar answered Nov 02 '22 20:11

Bryan


You're talking about two different things:

  1. From a design a best-practices perspective, should you always dispose an object for which you're responsible? Yes
  2. Are you going to experience a memory leak following the pattern you show in the second example? No, if for no other reason than the fact that MemoryStream.Dispose doesn't actually do anything.
like image 2
Adam Robinson Avatar answered Nov 02 '22 18:11

Adam Robinson