Should I implement IDisposable when class has IDisposable member but no unmanaged resources?

Question

The MSDN documentation and many answers here on StackOverflow go to lengths to disucss correctly implementing IDisposable, e.g. MSDN IDisposable, MSDN Implementing IDisposable, An excellent StackOverflow Q&A

However none of them seem to cover a more common use-case I have: what to do when my class has an IDisposable member that lives longer than one method? For example

  class FantasticFileService
  {
    private FileSystemWatcher fileWatch; // FileSystemWatcher is IDisposable

    public FantasticFileService(string path)
    {
      fileWatch = new FileSystemWatcher(path);
      fileWatch.Changed += OnFileChanged;
    }

    private void OnFileChanged(object sender, FileSystemEventArgs e)
    {
      // blah blah
    }
  }

The closest MSDN gets to addressing this problem only covers the use-case when the instance of IDisposable is short lived so says call Dispose e.g. by using using:

Implement IDisposable only if you are using unmanaged resources directly. If your app simply uses an object that implements IDisposable, don't provide an IDisposable implementation. Instead, you should call the object's IDisposable.Dispose implementation when you are finished using it.

of course that is not possible here where we need the instance to survive longer than a method call!?

I suspect the correct way to do this would be to implement IDisposable (passing the responsibility to creator of my class to dispose it) but without all finalizer and protected virtual void Dispose(bool disposing) logic becuase I don't have any unmanged resources, i.e.:

  class FantasticFileService : IDisposable
  {
    private FileSystemWatcher fileWatch; // FileSystemWatcher is IDisposable

    public FantasticFileService(string watch)
    {
      fileWatch = new FileSystemWatcher(watch);
      fileWatch.Changed += OnFileChanged;
    }

    public void Dispose()
    {
      fileWatch.Dispose();
    }
  }

But why is this use-case not explicitly covered in any official documentation? And the fact it explicitly says do not implement IDisposable if your class does not have unmanaged resources makes me hesitant to do so... What is a poor programmer to do?

Your second link [Implementing a Dispose Method](https://msdn.microsoft.com/en-us/library/fs2xkftw(v=vs.110).aspx) explains it where they say _"Free any other managed objects here"_. Also I'm not sure whether you have implemented `Dispose()` correctly. Aren't you missing `GC.SuppressFinalize(this);`? — , Jun 21 '16 at 02:07
"Also I'm not sure whether you have implemented Dispose() correctly", Quite likely! The question is _how to implement dispose correctly_ in this specific scenario, I wouldn't be asking if I knew! — markmnl, Jun 21 '16 at 02:10
Could not find good link on "*yes* you should implement IDsiposable in this case", but I don't see any other way that is generally agreed upon to specify that your object requires explicit cleanup at the end. Note that you still need to go for full Dispose pattern unless your class is `sealed` (http://stackoverflow.com/questions/3882804/finalizer-and-idisposable) — Alexei Levenkov, Jun 21 '16 at 02:16

gnalck · Accepted Answer · 2016-06-21T02:20:15.360

25

It looks like your case is indeed covered by some documentation, namely the design warning CA1001: Types that own disposable fields should be disposable.

That link has an example of what your IDisposable implementation should look like. It will be something like as follows. Eventual design guidelines can be found at CA1063: Implement IDisposable correctly.

  class FantasticFileService : IDisposable
  {
    private FileSystemWatcher fileWatch; // FileSystemWatcher is IDisposable

    public FantasticFileService(string watch)
    {
      fileWatch = new FileSystemWatcher(watch);
      fileWatch.Changed += OnFileChanged;
    }

    ~FantasticFileService()
    {
      Dispose(false);
    }

    protected virtual void Dispose(bool disposing)
    {
      if (disposing && fileWatch != null)
      {
        fileWatch.Dispose();
        fileWatch = null;
      }
    }

    public void Dispose()
    {
      Dispose(true);
      GC.SuppressFinalize(this);
    }
  }

edited Jun 21 '16 at 02:20

answered Jun 21 '16 at 02:11

gnalck

972
6
12

Additionally, looks like a similar question/answer over at http://stackoverflow.com/questions/33508669/disposing-of-filesystemwatcher – gnalck Jun 21 '16 at 02:16
Thanks I like the official link and the fact it explicitly talks about my scenario. Does look similar however I just happened to use FileSystemWatcher as an example, it could be any IDisposable – markmnl Jun 21 '16 at 02:19
Missing ~FantasticFileService() – Kason Jun 21 '16 at 02:19
@Kason aww, thought I would be able to get an edit in before anyone noticed :). nice catch! – gnalck Jun 21 '16 at 02:21
1

Though, for posterity's sake, it's worth noting that adding the destructor doesnt change the functionality of the code at all. Also see comments @ http://stackoverflow.com/a/628814/6137718 – gnalck Jun 21 '16 at 02:28
1

Indeed the linked page says: "If the class does not directly own any unmanaged resources, it should not implement a finalizer." – markmnl Jun 22 '16 at 05:31
No need of a Finalizer `~FantasticFileService()` since the class doesn't own an unmanaged resource directly according to the document of CA1001 – Robert May 12 '19 at 03:37
checking/setting to null is problematic for non nullable types etc, you also need to use a `disposed` flag – kofifus Nov 18 '21 at 23:59

score 4 · Answer 2 · answered Jun 21 '16 at 02:15

As you have gathered, you need to make FantasticFileService:IDisposable disposable too. Dispose() can be used to get rid of managed resources as well as unmanaged.

Try something like this:

class FantasticFileService:IDisposable
{
    private FileSystemWatcher fileWatch; // FileSystemWatcher is IDisposable
    private bool disposed;

    public FantasticFileService(string path)
    {
        fileWatch = new FileSystemWatcher(path);
        fileWatch.Changed += OnFileChanged;
    }

    private void OnFileChanged(object sender, FileSystemEventArgs e)
    {
        // blah blah
    }

    // Public implementation of Dispose pattern callable by consumers.
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    // Protected implementation of Dispose pattern.
    protected virtual void Dispose(bool disposing)
    {
        if (disposed)
            return;

        if (disposing)
        {
            if (fileWatch != null)
            {
                fileWatch.Dispose();
                fileWatch = null;                   
            }
            // Free any other managed objects here.
            //
        }

        // Free any unmanaged objects here.
        //
        disposed = true;
    }

    ~FantasticFileService()
    {
        Dispose(false);
    }
}

Should I implement IDisposable when class has IDisposable member but no unmanaged resources?

2 Answers2

See also

Linked

Related