Правильное переопределение Dispose(bool disposing)?

232

18 октября 2017, 04:05

Имеется класс, реализующий паттерн Disposable:

    internal class SomeDisposableClass : IDisposable
    {
        private readonly System.IO.Stream _managedResource;
        private bool _disposed;
        public SomeDisposableClass(System.IO.Stream managedResource)
        {
            _managedResource = managedResource;
        }
        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }
        protected virtual void Dispose(bool disposing)
        {
            if(_disposed)
            {
                return;
            }
            if(disposing)
            {
                _managedResource?.Dispose();
            }
            _disposed = true;
        }
    }

А есть класс, наследующийся от него:

    internal sealed class DerivedDisposableClass : SomeDisposableClass
    {
        private readonly System.IO.Stream _anotherResource;
        public DerivedDisposableClass(System.IO.Stream managedResource, System.IO.Stream anotherResource) 
            : base(managedResource)
        {
            _anotherResource = anotherResource;
        }
        protected override void Dispose(bool disposing)
        {
            _anotherResource?.Dispose();
            base.Dispose(disposing);
        }
    }

Вопрос: Нужно ли в классе-наследнике вводить флаг _disposed? Если да, то какой от этого профит?

И ещё: Может кто-нибудь на пальцах объяснить, какова польза от GC.SuppressFinalize(this) в классе-родителе (при условии, что наследник будет иметь финализатор)?

Answer 1

Флаг _disposed в вашем варианте наследнику нужен потому что как ему иначе определить что его уже нет?

Чтобы не заводить отдельный флаг на каждого наследника - можно немного изменить родителя.

Вариант 1. Вынесите проверку флага наружу, в невиртуальный Dispose(). В таком случае наследник будет просто знать что переопределенный Dispose(bool) будет вызван только один раз и проверка будет не нужна.

Вариант 2. Сделайте защищенное (protected) свойство IsDisposed в базовом классе.

От GC.SuppressFinalize(this) большая польза - он отменяет финализатор, что позволяет сборщику мусора собрать ваш объект за одну попытку а не за две.

Тем не менее, полный паттерн Disposable давно уже устарел. Гораздо красивее выглядит альтернативный подход, при котором считается недопустимым владение одновременно и управляемым и неуправляемым ресурсом в одном классе.

Недостатки классического паттерна Disposable:

он создает избыточную нагрузку на память потому что "тяжелые" объекты с финализаторами не могут быть собраны GC пока финализатор не отработает или не будет отменен;
он вызывает утечку ресурсов при выгрузке домена приложений (а это частое явление в том же ASP.NET).

Согласно более современному подходу, обычные классы в принципе не могут владеть неуправляемыми ресурсами - а потому им не нужен ни финализатор, ни метод Dispose(bool).

Для неуправляемых же ресурсов полагается делать управляемые обертки, желательно - на основе SafeHandle, но можно и на основе CriticalFinalizerObject.

Более подробно этот подход расписал VladD в вопросе Как и когда нужно имплементировать IDisposable?

Answer 2

Для начала, ваша имплементация неверна: вы не должны работать с управляемыми ресурсами в финализаторе. Соответственно вам нужно

if(disposing)
{
    _anotherResource?.Dispose();
}

Флаг _disposed имеет смысл вводить, если он не доступен из базового класса. Если флаг доступен из базового класса, по идее можно его и не перекрывать.

Флаг понадобится вам для того, чтобы бросить исключение, если публичный метод будет вызван на уже Dispose-нутом объекте. А также для того, чтобы сделать ваш метод Dispose повторяемым: два вызова Dispose подряд не должны приводить к падению, и должны быть равносильны одному. (В принципе, часто для этой цели можно обойтись и без флага, да.)

По поводу SuppressFinalize смотрите ответ @Pavel Mayorov. (А также обязательно прочитайте вторую часть ответа, под чертой.)