0

So I have a class which implements IDisposable, and I have several methods (in another class) which follow the pattern below:

public void SomeMethod()
{
    DisposableObject disposableObject = new DisposableObject();

    // Do some stuff with the object

    SomeOtherMethod(disposableObject);
    disposableObject.Dispose();
}

While all of these methods do different things, they all call SomeOtherMethod at the end, which does a few more things with the disposable object before it's no longer needed. When I move disposableObject.Dispose(); into SomeOtherMethod, Visual Studio gives me a message saying: "Use recommended dispose pattern to ensure that object created by 'new DisposableObject()' is disposed on all paths: using statement/declaration or try/finally"

This message appears regardless of whether or not I pass the disposable object to SomeOtherMethod using the ref keyword.

My question is, will that object be disposed as long as SomeOtherMethod calls Dispose() on it? I'm assuming it will, and Visual Studio continues to send the message simply because it isn't "aware" of what's happening to that object in subsequent methods, but I'd love to get some confirmation on that!

  • Whichever *scope* allocates the object owns it and should be what disposes it. Simple. Put a `using` block on it. Using "transfer semantics", i.e. giving ownership to `SomeOtherMethod` and havig it dispose the object, will likely lead to confusion and is best avoided. – madreflection Sep 28 '19 at 06:55
  • That is IDE0067, a code analysis warning. It is trying to tell you that the object might not be disposed if the method throws an exception. Although code analysis usually has trouble generating good diagnostics about IDisposable usage, it is probably correct about this one. You do favor the *using* statement here. Disposing it in the called method would only be wise if that method takes a very long time to execute after the Dispose() call. – Hans Passant Sep 28 '19 at 07:17

3 Answers3

1

It may be disposed or may be not, depends on the fact whether the execution will reach the Dispose invocation or not and that's because an exception can be thrown before the Dispose is called. Using try finally construction explicitly or implicitly by using keyword ensures that it will be called for any scenario and that's why VS gives you the warning.

Dmytro Mukalov
  • 1,949
  • 1
  • 9
  • 14
0

No matter where one call the Dispose(), it is called.

Not using the language keyword using for the disposable pattern, therefore moving the dispose in another method, is an anti-pattern, therefore it is a bad practice and a source of potential problems.

You can only remove the warning by adding the warning number in the project build settings.

The method Dispose() doesn't destroy the object.

The dispose pattern is only for freeing unmanaged resources like windows handle and shared memories.

After a call to Dispose() you still have the object and so the reference to the object that remains referenced in the managed memory.

Dispose() is made to be called once time at the end of object usage and no more.

The compiler send you a warning because you break the standard behavior of the pattern usage that is to use the using keyword.

And breaking standards can be source of problems.

The using of disposable objects standard is made to avoid bugs by letting the compiler generates the try { ... } finally { Dispose() } block to be sure that Dispose() is correctly called in the right place to avoid mistakes.

So avoid calling the Dispose() directly.

Unless you are sure of what you do, prefer using:

public void SomeMethod()
{
  using ( DisposableObject disposableObject = new DisposableObject() )
  {
    // Do some stuff with the object
    SomeOtherMethod(disposableObject);
  }
}

And your code may be robust.

  • 2
    "`Dispose()` is made to be called once time at the end of object usage **and no more.**". Not agree with "no more" part. `Dispose` method should be [**idempotent**](https://stackoverflow.com/questions/8923853/should-idisposable-dispose-implementations-be-idempotent) – Artur Sep 28 '19 at 09:18
  • @Artur I agree... If the Dispose implementation is done properly... But do not consider that this is always the case because in computer science one should never believe that an implementation is able to do something if the specification does not quote it explicitly... `IDispose` is an interface without implementation available for all coders. The .NET Framework classes certainly done the things properly, but anyone can implement a good or a bad `Dispose()` for himself or for the `IDispose` and the `using` keyword... Hence the existence of the pattern and the compiler warning. –  Sep 28 '19 at 09:43
  • 1
    I'm not talking about if the rule is always satisfied or not, I'm saying that you stated wrong statement like **it is** a rule. But the main problem with you answer is that it not answering the question at all. The OP asked about moving `Dispose` call into the `SomeOtherMethod` and not about the "proper way to implement Disposable pattern". – Artur Sep 28 '19 at 12:13
  • The OP is annoyed by the compiler warning. No matter where one call the dispose, it is called. I answered that not using the language keyword `using` for the disposable pattern, therefore moving the dispose in another method, is an anti-pattern, therefore it is a bad practice and a source of potential problems. The OP can only remove the warning by adding the warning number in the project build settings. –  Sep 28 '19 at 12:26
0

will that object be disposed

Sorry, but that’s a meaningless question. The CLR does not keep track of whether an object has had its “dispose” method called or not (see Will the Garbage Collector call IDisposable.Dispose for me? )

As a general rule, it is always much nicer (readable/ maintainable / less-bug-prone / etc) that a method that creates an issue should also be the one that cleans up after itself. As you’ve just found, this pattern also allows automated checking by the compiler - and again, it is also a good rule to ensure that your code compiles cleanly without errors OR WARNINGS.

In this case, the warning is giving you a couple of ways to implement this cleanly; personally, I would prefer the “using” clause (so avoiding having to have an explicit call to “dispose”) like :

 public void SomeMethod()
 {
     using (DisposableObject disposableObject = new DisposableObject() )
      {

           // Do some stuff with the object

           SomeOtherMethod(disposableObject);
       }

}

racraman
  • 4,988
  • 1
  • 16
  • 16
  • Thanks, this is more or less the answer I was looking for. All of the answers pointed out essentially the same thing, but this made the most sense to me. – Tom Weiland Sep 28 '19 at 19:27