views:

33

answers:

1

Consider the following test snippet:

        // act
        AutoResetEvent workDoneEvent = new AutoResetEvent(false);
        ThreadPool.QueueUserWorkItem(delegate
                                         {
                                             ProcessAndSignal(processor, workDoneEvent);
                                         }, null);

        // let worker thread have a go
        workDoneEvent.WaitOne();
        blockingFetcher.WaitForNextMessage = false;

        // assert
        Assert.That(processor.StopCause, Is.Null);
    }

    private static void ProcessAndSignal(MessageProcessor processor, AutoResetEvent workDoneEvent)
    {
        workDoneEvent.Set();
        // this invocation will block until the WaitForNextMessageFlag is set
        processor.ProcessMessages();
    }

Ideal scenario:

  1. ProcessAndSignalMethod is queued on the thread pool but does not start to execute.
  2. The main thread blocks (autoResetEvent.WaitOne())
  3. A worker thread starts to execute the "ProcessAndSignal" method
  4. The worker threads has enough time to signal the flag and start execution of the ProcessMessages method
  5. The main thread is spawned back into life and sets the property which will cause the ProcessAndSignal method to complete gracefully

Can the following scenario occur?

1) ProcessAndSignal() will start to execute before the main thread sets the AutoResetEvent to WaitOne() which will cause a deadlock (the processor.ProcessMessages() will go into an infinitive loop)

+1  A: 

Yes, the scenario can occur. Yes it can deadlock if you don't declare the bool variable as volatile. Just don't use a bool, use an event like you did.

The logic looks weird, it smells like you are trying to let the main thread wait for the processing to be completed. The workDoneEvent doesn't actually signal that the work was done. Right now the main thread will check the assert before the worker is done, that can't be good. If the intention was that it signals that the worker is done then ProcessAndSignal should be the one calling Set(), at the end of the method. And the main thread should call WaitOne().

If this is at all accurate then you just should not use QUWI, just call ProcessAndSignal directly without using a thread. Far more efficient, zero odds for threading problems.

Hans Passant