Безопасно ли сигнализировать и немедленно закрыть ManualResetEvent?

Question

Я чувствую, что должен знать ответ на этот вопрос, но я все равно спрошу на всякий случай, если я совершаю потенциально катастрофическую ошибку.

Следующий код выполняется должным образом без ошибок / исключений:

static void Main(string[] args)
{
    ManualResetEvent flag = new ManualResetEvent(false);
    ThreadPool.QueueUserWorkItem(s =>
    {
        flag.WaitOne();
        Console.WriteLine("Work Item 1 Executed");
    });
    ThreadPool.QueueUserWorkItem(s =>
    {
        flag.WaitOne();
        Console.WriteLine("Work Item 2 Executed");
    });
    Thread.Sleep(1000);
    flag.Set();
    flag.Close();
    Console.WriteLine("Finished");
}

Конечно, как это обычно бывает с многопоточным кодом, успешное тестирование не доказывает, что это на самом деле потокобезопасно. Тест также будет успешным, если я поставлю Close до Set, хотя в документации четко указано, что попытка сделать что-либо после Close приведет к неопределенному поведению.

Мой вопрос, когда я призываю ManualResetEvent.Set метод, гарантированно ли будет сигнализировать все ожидающие потоки перед возвратом контроля вызывающей стороне? Другими словами, при условии, что я могу гарантировать, что больше не будет звонков WaitOne, безопасно ли здесь закрывать дескриптор, или возможно, что при некоторых обстоятельствах этот код будет препятствовать тому, чтобы официанты получили сигнал, или приведет к ObjectDisposedException?

Документация только говорит, что Set переводит его в "сигнальное состояние" - он, кажется, не претендует на то, когда официанты действительно получат этот сигнал, поэтому я хотел бы быть уверен.

Answer 1

Когда вы сигнализируете с ManualResetEvent.Set вам гарантировано, что все потоки, которые ожидают этого события (т.е. находятся в состоянии блокировки на flag.WaitOne) будет сигнализироваться перед возвратом контроля вызывающей стороне.

Конечно, есть обстоятельства, когда вы можете установить флаг, и ваш поток не увидит его, потому что он выполняет некоторую работу, прежде чем проверяет флаг (или, как подсказывает nobugs, если вы создаете несколько потоков):

ThreadPool.QueueUserWorkItem(s =>
{
    QueryDB();
    flag.WaitOne();
    Console.WriteLine("Work Item 1 Executed");
});

Есть флаг на флаге, и теперь вы можете создать неопределенное поведение при его закрытии. Ваш флаг является общим ресурсом между вашими потоками, вы должны создать защелку обратного отсчета, по которой каждый поток сигнализирует о завершении. Это устранит раздор на вашем flag,

public class CountdownLatch
{
    private int m_remain;
    private EventWaitHandle m_event;

    public CountdownLatch(int count)
    {
        Reset(count);
    }

    public void Reset(int count)
    {
        if (count < 0)
            throw new ArgumentOutOfRangeException();
        m_remain = count;
        m_event = new ManualResetEvent(false);
        if (m_remain == 0)
        {
            m_event.Set();
        }
    }

    public void Signal()
    {
        // The last thread to signal also sets the event.
        if (Interlocked.Decrement(ref m_remain) == 0)
            m_event.Set();
    }

    public void Wait()
    {
        m_event.WaitOne();
    }
}

1. Каждый поток сигнализирует о защелке обратного отсчета.
2. Ваш главный поток ожидает защелки обратного отсчета.
3. Основной поток очищается после сигналов защелки обратного отсчета.

В конечном счете, время, когда вы спите в конце, НЕ является безопасным способом решения ваших проблем, вместо этого вы должны разработать свою программу так, чтобы она была на 100% безопасна в многопоточной среде.

ОБНОВЛЕНИЕ: Единственный Производитель / Многократные Потребители
Предполагается, что ваш производитель знает, сколько потребителей будет создано. После создания всех ваших потребителей вы сбрасываете CountdownLatch с заданным количеством потребителей:

// In the Producer
ManualResetEvent flag = new ManualResetEvent(false);
CountdownLatch countdown = new CountdownLatch(0);
int numConsumers = 0;
while(hasMoreWork)
{
    Consumer consumer = new Consumer(coutndown, flag);
    // Create a new thread with each consumer
    numConsumers++;
}
countdown.Reset(numConsumers);
flag.Set();
countdown.Wait();// your producer waits for all of the consumers to finish
flag.Close();// cleanup

Answer 2

Это не хорошо. Вам повезло, потому что вы только начали две темы. Они сразу начнут работать, когда вы вызовете Set на двухъядерном компьютере. Попробуйте вместо этого и посмотрите, как бомба:

    static void Main(string[] args) {
        ManualResetEvent flag = new ManualResetEvent(false);
        for (int ix = 0; ix < 10; ++ix) {
            ThreadPool.QueueUserWorkItem(s => {
                flag.WaitOne();
                Console.WriteLine("Work Item Executed");
            });
        }
        Thread.Sleep(1000);
        flag.Set();
        flag.Close();
        Console.WriteLine("Finished");
        Console.ReadLine();
    }

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

Answer 3

Для меня это выглядит рискованно, даже если из-за [текущей] реализации все в порядке. Вы пытаетесь утилизировать ресурс, который все еще может использоваться.

Это похоже на создание и создание объекта и его слепое удаление даже до того, как потребители этого объекта будут готовы.

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

Учитывая, что вам все равно придется ждать других потоков, вы можете с тем же успехом потом почистить.

Answer 4

Я считаю, что есть состояние гонки. Написав объекты событий на основе условных переменных, вы получите код, подобный следующему:

mutex.lock();
while (!signalled)
    condition_variable.wait(mutex);
mutex.unlock();

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

Согласно документации на Close, это только освобождает неуправляемые ресурсы. Так что, если событие использует только управляемые ресурсы, вам может повезти. Но это может измениться в будущем, поэтому я буду ошибаться в сторону предосторожности и не закрою событие, пока вы не узнаете, что оно больше не используется.