Как узнать, когда безопасно вызывать Dispose?

У меня есть приложение для поиска, которому требуется некоторое время (от 10 до 15 секунд), чтобы вернуть результаты для некоторых запросов. Нередко есть несколько одновременных запросов на одну и ту же информацию. В нынешнем виде я должен обрабатывать их независимо, что делает довольно много ненужной обработки.

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

Каждый запрос имеет ключ, который идентифицирует запрашиваемые данные. Я поддерживаю словарь запросов, определяемых ключом запроса. Объект запроса имеет некоторую информацию о состоянии и WaitHandle, который используется для ожидания результатов.

Когда клиент вызывает мой метод Search, код проверяет словарь, чтобы увидеть, существует ли уже запрос для этого ключа. Если это так, клиент просто ждет WaitHandle. Если запроса не существует, я создаю его, добавляю в словарь и выполняю асинхронный вызов для получения информации. Опять же, код ожидает события.

Когда асинхронный процесс получает результаты, он обновляет объект запроса, удаляет запрос из словаря и сигнализирует о событии.

Все это прекрасно работает. За исключением того, что я не знаю, когда избавиться от объекта запроса. То есть, поскольку я не знаю, когда его использует последний клиент, я не могу вызвать на нем Dispose. Приходится ждать, когда приедет сборщик мусора и уберет.

Вот код:

class SearchRequest: IDisposable
{
    public readonly string RequestKey;
    public string Results { get; set; }
    public ManualResetEvent WaitEvent { get; private set; }

    public SearchRequest(string key)
    {
        RequestKey = key;
        WaitEvent = new ManualResetEvent(false);
    }

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

ConcurrentDictionary<string, SearchRequest> Requests = new ConcurrentDictionary<string, SearchRequest>();

string Search(string key)
{
    SearchRequest req;
    bool addedNew = false;
    req = Requests.GetOrAdd(key, (s) =>
        {
            // Create a new request.
            var r = new SearchRequest(s);
            Console.WriteLine("Added new request with key {0}", key);
            addedNew = true;
            return r;
        });

    if (addedNew)
    {
        // A new request was created.
        // Start a search.
        ThreadPool.QueueUserWorkItem((obj) =>
            {
                // Get the results
                req.Results = DoSearch(req.RequestKey);  // DoSearch takes several seconds

                // Remove the request from the pending list
                SearchRequest trash;
                Requests.TryRemove(req.RequestKey, out trash);

                // And signal that the request is finished
                req.WaitEvent.Set();
            });
    }

    Console.WriteLine("Waiting for results from request with key {0}", key);
    req.WaitEvent.WaitOne();
    return req.Results;
}

В принципе, я не знаю, когда будет выпущен последний клиент. Как бы я ни нарезал здесь, у меня есть состояние гонки. Рассмотреть возможность:

  1. Поток A Создает новый запрос, запускает поток 2 и ожидает обработки дескриптора ожидания.
  2. Поток B Начинает обработку запроса.
  3. Поток C обнаруживает ожидающий запрос, а затем выгружается.
  4. Поток B Завершает запрос, удаляет элемент из словаря и устанавливает событие.
  5. Ожидание потока А удовлетворено, и он возвращает результат.
  6. Поток C просыпается, вызывает WaitOne, освобождается и возвращает результат.

Если я использую какой-то подсчет ссылок, чтобы «последний» клиент вызывал Dispose, тогда объект будет удален потоком A в приведенном выше сценарии. Затем поток C умрет, когда попытается дождаться удаленного WaitHandle.

Я вижу единственный способ исправить это — использовать схему подсчета ссылок и защитить доступ к словарю блокировкой (в этом случае использование ConcurrentDictionary бессмысленно), чтобы поиск всегда сопровождался увеличением счетчика ссылок. Принимая во внимание, что это сработает, это кажется уродливым взломом.

Другим решением было бы отказаться от WaitHandle и использовать механизм, подобный событию, с обратными вызовами. Но это также потребует от меня защиты поиска с помощью блокировки, и у меня есть дополнительные сложности, связанные с работой с событием или открытым делегатом многоадресной рассылки. Это тоже похоже на взлом.

Это, вероятно, не проблема в настоящее время, потому что это приложение еще не получает достаточно трафика для этих заброшенных дескрипторов, чтобы суммироваться до того, как придет следующий проход GC и очистит их. А может быть, это никогда не будет проблемой? Однако меня беспокоит то, что я оставляю их на очистку сборщику мусора, когда мне нужно позвонить Dispose, чтобы избавиться от них.

Идеи? Это потенциальная проблема? Если да, то есть ли у вас чистое решение?


person Jim Mischel    schedule 16.09.2011    source источник
comment
одно слово приходит на ум: Иииии!!   -  person Mitch Wheat    schedule 16.09.2011
comment
@Mitch: что такое Иииии в ответ? Дизайн? Или проблема?   -  person Jim Mischel    schedule 16.09.2011
comment
@Jim Mischel - Вы должны иметь в виду, что ConcurrentDictionary имеет побочные эффекты. Если несколько потоков попытаются вызвать GetOrAdd одновременно, то фабрика может быть вызвана для всех из них, но победит только один. Значения, созданные для других потоков, будут просто отброшены, однако к тому времени вычисления будут завершены.   -  person Enigmativity    schedule 16.09.2011
comment
@ Джим Мишель. Также вы упомянули, что сборщик мусора выполнит вашу очистку, но сборщик мусора никогда не вызывает .Dispose() - вам всегда нужно явно вызывать .Dispose() в своем коде.   -  person Enigmativity    schedule 16.09.2011
comment
@Enigmativity: я знаю, что сборщик мусора не вызывает Dispose, но выполняет финализатор. И финализатор для ManualResetEvent вызывает Dispose, что освобождает дескриптор операционной системы.   -  person Jim Mischel    schedule 16.09.2011
comment
@Enigmativity: то, что ConcurrentDictionary будет вызывать фабрику в нескольких потоках для одного и того же ключа, вызывает некоторое разочарование. Мне определенно придется повторно посетить эту часть решения. Спасибо.   -  person Jim Mischel    schedule 16.09.2011


Ответы (2)


Возможно, стоит использовать Lazy<T> вместо SearchRequest.Results? Но это, вероятно, повлечет за собой небольшой редизайн. Не продумал это полностью.

Но то, что, вероятно, было бы почти заменой для вашего варианта использования, — это реализация ваших собственных методов Wait() и Set() в SearchRequest. Что-то типа:

object _resultLock;

void Wait()
{
  lock(_resultLock)
  {
     while (!_hasResult)
       Monitor.Wait(_resultLock);
  }
}

void Set(string results)
{
  lock(_resultLock)
  {
     Results = results;
     _hasResult = true;
     Monitor.PulseAll(_resultLock);
  }
}

Не надо утилизировать. :)

person Ilian    schedule 16.09.2011
comment
Мне нравится это решение. Нужно немного подумать, но выглядит неплохо. И мне действительно не нужен _hasResult. Я могу просто использовать Result как в while (Result == null). - person Jim Mischel; 16.09.2011

Я думаю, что лучший способ заставить это работать — использовать TPL для всех ваших потребностей в многопоточности. Это то, в чем он хорош.

Согласно моему комментарию к вашему вопросу, вы должны иметь в виду, что ConcurrentDictionary имеет побочные эффекты. Если несколько потоков попытаются вызвать GetOrAdd одновременно, то фабрика может быть вызвана для всех из них, но победит только один. Значения, созданные для других потоков, будут просто отброшены, однако к тому времени вычисления будут завершены.

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

Итак, вот что я предлагаю:

private Dictionary<string, Task<string>> _requests
    = new Dictionary<string, Task<string>>();

public string Search(string key)
{
    Task<string> task;
    lock (_requests)
    {
        if (_requests.ContainsKey(key))
        {
            task = _requests[key];
        }
        else
        {
            task = Task<string>
                .Factory
                .StartNew(() => DoSearch(key));
            _requests[key] = task;
            task.ContinueWith(t =>
            {
                lock(_requests)
                {
                    _requests.Remove(key);
                }
            });
        }
    }
    return task.Result;
}

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

Я проверил код, и он работает.

person Enigmativity    schedule 16.09.2011
comment
+1. Да, это выглядит очень хорошо. Спасибо. Я добавлю это в свой проект и посмотрю, как это пойдет. Очевидно, мне нужно немного больше узнать о TPL. - person Jim Mischel; 16.09.2011
comment
Да, вся эта история с GetOrAdd звонками на фабрику несколько раз - это странно. Эта особенность преследовала меня раньше. - person Brian Gideon; 16.09.2011
comment
На самом деле я использовал другое предложение, поскольку я уже выполнялся в фоновом потоке, и не было веской причины запускать еще один. Но ваше предложение заставило меня переосмыслить дизайн моего приложения и позволило в значительной степени упростить его. Спасибо за предложение. - person Jim Mischel; 17.09.2011