Как правильно добавить безопасность потока в объект IDisposable? - PullRequest
Новая
       30

Как правильно добавить безопасность потока в объект IDisposable?

27 голосов
/ 19 января 2012

Представьте себе реализацию интерфейса IDisposable, который имеет несколько открытых методов.

Если экземпляр этого типа совместно используется несколькими потоками, и один из потоков может его утилизировать, каков наилучший способ гарантировать, что другие потоки не будут пытаться работать с экземпляром после удаления? В большинстве случаев после удаления объекта его методы должны знать об этом и выдавать ObjectDisposedException или, возможно, InvalidOperationException или, по крайней мере, сообщать вызывающему коду о том, что он что-то делает не так. Нужна ли синхронизация для каждого метода - особенно при проверке его утилизации? Все ли реализации IDisposable с другими открытыми методами должны быть поточно-ориентированными?

Вот пример: 

public class DummyDisposable : IDisposable
{
    private bool _disposed = false;

    public void Dispose()
    {
        _disposed = true;
        // actual dispose logic
    }

    public void DoSomething()
    {
        // maybe synchronize around the if block?
        if (_disposed)
        {
            throw new ObjectDisposedException("The current instance has been disposed!");
        }

        // DoSomething logic
    }

    public void DoSomethingElse()
    {
         // Same sync logic as in DoSomething() again?
    }
}

Ответы [ 6 ]

13 голосов
/ 19 января 2012

Большинство реализаций BCL для Dispose не являются поточно-ориентированными. Идея состоит в том, что вызывающий Dispose должен убедиться, что никто больше не использует этот экземпляр до его удаления. Другими словами, это увеличивает ответственность за синхронизацию. Это имеет смысл, так как в противном случае теперь все ваши другие потребители должны обрабатывать граничный случай, когда объект был расположен, пока они его использовали.

Тем не менее, если вы хотите поточно-ориентированный класс Disposable, вы можете просто создать блокировку вокруг каждого открытого метода (включая Dispose) с проверкой на _disposed вверху. Это может стать более сложным, если у вас есть долгосрочные методы, в которых вы не хотите удерживать блокировку для всего метода.

11 голосов
/ 19 января 2012

Самое простое, что вы можете сделать, - пометить закрытую переменную как volatile и проверить ее в начале ваших методов.Затем вы можете бросить ObjectDisposedException, если объект уже был утилизирован.

Есть два предостережения:

  1. Вы не должны бросать ObjectDisposedException, если метод является обработчиком событий.Вместо этого вы должны просто изящно выйти из метода, если это возможно.Причина в том, что существует состояние гонки, когда события могут быть вызваны после того, как вы отмените подписку на них.(См. эту статью Эрика Липперта для получения дополнительной информации.)

  2. Это не мешает вашему классу быть расположенным, пока вы выполняете одно из следующих действий:ваши методы класса.Поэтому, если в вашем классе есть члены экземпляра, к которым нельзя получить доступ после утилизации, вам потребуется настроить некоторые блокировки, чтобы обеспечить контроль доступа к этим ресурсам.

Руководство Microsoftabout IDisposable говорит, что вы должны проверять наличие всех методов, но я лично не нашел в этом необходимости.На самом деле вопрос в том, будет ли что-то вызывать исключение или вызывать непреднамеренные побочные эффекты, если вы разрешите выполнение метода после удаления класса.Если ответ «да», вам нужно проделать определенную работу, чтобы этого не произошло.

С точки зрения того, должны ли все классы IDisposable быть потокобезопасными: Нет. Большинство сценариев использования для одноразовых классов включаютдоступ к ним возможен только из одного потока.

При этом вам, возможно, захочется выяснить, почему вам нужен одноразовый класс для обеспечения безопасности потоков, поскольку он добавляет много дополнительной сложности.Может быть альтернативная реализация, которая позволяет вам не беспокоиться о проблемах безопасности потоков в вашем одноразовом классе.

8 голосов
/ 19 января 2012

Я склонен использовать целое число, а не логическое значение в качестве поля для хранения удаленного состояния, потому что тогда вы можете использовать поточно-безопасный класс Interlocked, чтобы проверить, был ли вызван Dispose.

Что-то вродеthis: 

private volatile int _disposeCount;

public void Dispose()
{
    if (Interlocked.Increment(ref _disposeCount) == 1)
    {
        // disposal code here
    }
}

Это гарантирует, что код удаления вызывается только один раз, независимо от того, сколько раз вызывается метод, и полностью безопасен для потоков.

Тогда каждый метод может просто использоватьвызовите этот метод как проверку барьера: 

private void ThrowIfDisposed()
{
   if (_disposeCount > 0) throw new ObjectDisposedException(GetType().Name);
}

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

Если вы только что имели в виду в отношении самой удаленной проверки - мой пример выше подойдет.

РЕДАКТИРОВАТЬ: ответить на комментарий "В чем разница между этим иФлаг volatile bool? Немного сбивает с толку наличие поля с именемthingCount, позволяющего ему хранить только значения 0 и 1 "

Volatile связано с обеспечением атомарности и безопасности операции чтения или записи.Это не делает процесс присвоения и проверки значения потокобезопасным.Так, например, следующее не является потокобезопасным, несмотря на изменчивость: 

private volatile bool _disposed;

public void Dispose()
{
    if (!_disposed)
    {
        _disposed = true

        // disposal code here
    }
}

Проблема здесь в том, что, если два потока были близко друг к другу, первый мог проверить _disposed, прочитать false, ввести блок кода ибыть выключенным перед установкой _disposed в true.Затем второй проверяет _disposed, видит false и также входит в блок кода.

Использование Interlocked гарантирует, что как назначение, так и последующее чтение являются одной атомарной операцией.

4 голосов
/ 19 января 2012

Я предпочитаю использовать целые числа и Interlocked.Exchange или Interlocked.CompareExchange для целочисленного типа объекта "disposed" или "state" variable; Я бы использовал enum, если бы Interlocked.Exchange или Interlocked.CompareExchange могли обрабатывать такие типы, но, увы, они не могут.

Одна вещь, о которой большинство дискуссий об IDisposable и финализаторах не упоминается, это то, что, хотя финализатор объекта не должен работать, пока IDisposable.Dispose () находится в процессе, у класса нет способа предотвратить объявление объектов своего типа умер, а затем воскрес. Конечно, если внешний код допускает это, очевидно, не может быть никаких требований, чтобы объект "работал нормально", но методы Dispose и finalize должны быть достаточно хорошо защищены, чтобы гарантировать, что они не повредят * 1008. * другие состояния объектов, которые, в свою очередь, обычно требуют использования либо блокировок, либо Interlocked операций с переменными состояния объекта.

1 голос
/ 19 января 2012

FWIW, ваш пример кода соответствует тому, как мои коллеги и я обычно решаем эту проблему.Обычно мы определяем приватный метод CheckDisposed для класса: 

private volatile bool isDisposed = false; // Set to true by Dispose

private void CheckDisposed()
{
    if (this.isDisposed)
    {
        throw new ObjectDisposedException("This instance has already been disposed.");
    }
}

Затем мы вызываем метод CheckDisposed() в начале всех открытых методов.

Если конфликт потоков из-за утилизации равенсчитается вероятным, а не ошибочным условием. Я также добавлю общедоступный метод IsDisposed() (аналогично Control.IsDisposed ).

Обновление: на основе комментариев относительнок значению превращения isDisposed volatile, обратите внимание, что проблема «забора» довольно тривиальна, учитывая то, как я использую метод CheckDisposed().По сути, это инструмент для устранения неполадок, позволяющий быстро отследить случай, когда код вызывает открытый метод для объекта после его удаления.Вызов CheckDisposed() в начале открытого метода никоим образом не гарантирует, что объект не будет размещен в этом методе.Если я считаю, что это риск, присущий дизайну моего класса, в отличие от состояния ошибки, которое я не учел, то я использую вышеупомянутый метод IsDisposed вместе с соответствующей блокировкой.

0 голосов
/ 19 января 2012

Вы должны заблокировать каждый доступ к ресурсам, которые вы собираетесь использовать. Я также добавил шаблон Dispose, который я обычно использую. 

public class MyThreadSafeClass : IDisposable
{
    private readonly object lockObj = new object();
    private MyRessource myRessource = new MyRessource();

    public void DoSomething()
    {
        Data data;
        lock (lockObj)
        {
            if (myResource == null) throw new ObjectDisposedException("");
            data = myResource.GetData();
        }
        // Do something with data
    }

    public void DoSomethingElse(Data data)
    {
        // Do something with data
        lock (lockObj)
        {
            if (myRessource == null) throw new ObjectDisposedException("");
            myRessource.SetData(data);
        }
    }

    ~MyThreadSafeClass()
    {
        Dispose(false);
    }
    public void Dispose() 
    { 
        Dispose(true); 
        GC.SuppressFinalize(this);
    }
    protected void Dispose(bool disposing) 
    {
        if (disposing)
        {
            lock (lockObj)
            {
                if (myRessource != null)
                {
                    myRessource.Dispose();
                    myRessource = null;
                }
            }
            //managed ressources
        }
        // unmanaged ressources
    }
}
...