Правильно ли это 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

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

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

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

Answer 2

EDIT Да, я неправильно интерпретировал часть о отделяя УТИЛИЗАЦИИ управляемых и неуправляемых ресурсов (все еще на первой чашки кофе).

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

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

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

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

Answer 3

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

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); 
    } 
}