Это правильная реализация IDisposable?

Question

Я никогда не могу вспомнить все правила реализации интерфейса IDisposable, поэтому я попытался создать базовый класс, который позаботится обо всем этом и сделает IDisposable простым в реализации. Я просто хотел услышать ваше мнение, если эта реализация в порядке, или вы видите что-то, что я мог бы улучшить. Предполагается, что пользователь этого базового класса наследует его, а затем реализует два абстрактных метода ReleaseManagedResources() и ReleaseUnmanagedResources(). Итак, вот код:

public abstract class Disposable : IDisposable
{
    private bool _isDisposed;
    private readonly object _disposeLock = new object();

    /// <summary>
    /// called by users of this class to free managed and unmanaged resources
    /// </summary>
    public void Dispose() {
        DisposeManagedAndUnmanagedResources();
    }

    /// <summary>
    /// finalizer is called by garbage collector to free unmanaged resources
    /// </summary>
    ~Disposable() { //finalizer of derived class will automatically call it's base finalizer
        DisposeUnmanagedResources();
    }

    private void DisposeManagedAndUnmanagedResources() {
        lock (_disposeLock) //make thread-safe
            if (!_isDisposed) { //make sure only called once
                try { //suppress exceptions
                    ReleaseManagedResources();
                    ReleaseUnmanagedResources();
                }
                finally {
                    GC.SuppressFinalize(this); //remove from finalization queue since cleaup already done, so it's not necessary the garbage collector to call Finalize() anymore
                    _isDisposed = true;
                }
            }
    }

    private void DisposeUnmanagedResources() {
        lock (_disposeLock) //make thread-safe since at least the finalizer runs in a different thread
            if (!_isDisposed) { //make sure only called once
                try { //suppress exceptions
                    ReleaseUnmanagedResources();
                }
                finally {
                    _isDisposed = true;
                }
            }
    }

    protected abstract void ReleaseManagedResources();
    protected abstract void ReleaseUnmanagedResources();

}

Answer 1

Я не могу комментировать правильность вашего кода, но я сомневаюсь, что вы обнаружите, что класс Disposable в основе вашего дерева наследования настолько гибок, насколько это необходимо. Бесполезно, если вы хотите унаследовать что-то, чем не владеете. Я думаю, что IDispoasble остается интерфейсом из-за множества разных доменов, в которых он может использоваться. Предоставление конкретной реализации у подножия иерархии наследования было бы ... ограничением.

Answer 2

Вы получаете доступ к управляемому объекту _disposeLock в финализаторе. Возможно, к тому времени он уже был собран. Не уверен, к чему это приведет, поскольку вы используете его только для блокировки.

Потоковая безопасность кажется излишним. Я не думаю, что вам нужно беспокоиться о конфликте между потоком GC и потоком вашего приложения.

В противном случае это выглядит правильно.

Answer 3

РЕДАКТИРОВАТЬ Да, я неправильно прочитал часть о разделении утилизации управляемых и неуправляемых ресурсов (все еще на первой чашке кофе).

Однако блокировка по-прежнему почти наверняка не нужна. Да, во время пассивного удаления код будет выполняться в потоке финализатора, который отличается от потока, в котором он изначально работал. Однако, если объект завершается таким способом, CLR уже определил, что нет никаких ссылок на этот объект и, следовательно, собрал его. Таким образом, нет другого места, которое может вызывать ваше распоряжение в то время, и, следовательно, нет причин для блокировки.

Пара других стилевых комментариев.

Зачем делать методы абстрактными? Тем самым вы заставляете производные классы реализовывать методы для удаления управляемых и неуправляемых ресурсов, даже если у них нет таких ресурсов. Правда нет смысла выводить из этого класса, если вам нечего распоряжаться. Но я думаю, что довольно распространено иметь только одно или другое, но не оба. Я бы сделал их виртуальными против абстрактных.

Также вы предотвращаете двойную утилизацию, но ничего не делаете, чтобы предупредить разработчика о том, что он дважды удаляет объект. Я понимаю, что документация MSDN гласит, что двойное удаление должно быть по существу запретом, но в то же время при каких обстоятельствах это должно быть законно? В общем, плохая идея получить доступ к объекту после его удаления. Двойное удаление требует повторного использования удаленного объекта и, вероятно, является ошибкой (это может произойти, если завершение не подавлено в активном удалении, но это также плохая идея). Если бы я реализовывал это, я бы выбрал двойную утилизацию, чтобы предупредить разработчика, что он использует объект неправильно (то есть использует его после того, как он уже был удален).

Answer 4

Если вас беспокоит безопасность потоков, я бы использовал облегченные операции с блокировками, а не тяжелые блокировки:

class MyClass: IDispose {
  int disposed = 0;

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

  public virtual void Dispose(bool disposing)
  {
     if (0 == Thread.InterlockedCompareExchange(ref disposed, 1, 0))
     {
       if (disposing)
       {
          // release managed resources here
       }
       // release unmanaged resources here
     }
  }

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