4

I want to start a thread that listens for events and handles them in the background. This is what I've done so far:

private Thread mThread;
private bool mKeepHandlingIt;

private void Init()
{
    mKeepHandlingIt = true;
    mThread = new Thread(ProcessEvents);
    mThread.SetApartmentState(ApartmentState.STA);
    mThread.Start();
}

private void ProcessEvents()
{
    StaticClass.CoolEvent += HandleIt;
    StaticClass.StartDoingStuff();

    while (mKeepHandlingIt)
    {
        Application.DoEvents();
    }

    StaticClass.StopDoingStuff();
    StaticClass.CoolEvent -= HandleIt;
}

private void HandleIt(object sender, EventArgs e)
{
    //Handles it
}

protected override void Dispose()
{
    if (mThread != null)
    {
        mKeepHandlingIt = false;
        mThread.Join();
    }
}

Is there a better approach to this? Like a thread class that would be better suited for this purpose? BackgroundWorker did not seem like a better choice in this case...

If this is a good approach, though, then I have a problem that I can't quite understand. The above solution works fine for listening to events and handling them, but when Dispose is called and mKeepHandlingIt is set to false, the while loop IS exited (debugger doesn't break if I place a breakpoint in the loop) but no code after the loop is executed and mThread.Join never returns. Basically... what I want to know is: how do I stop the thread and then make sure not to continue before it has cleaned up?

moggizx
  • 476
  • 1
  • 5
  • 19
  • I wouldn't use `Application.DoEvents` as it can lead to additional threads being started and strange things happening - (imagine a user clicks a button to start a process and then clicks it again because of `DoEvents` which starts the process again!). Since you are running stuff on a separate thread, you don't need it (the UI thread won't block). I'd consider using TPL/Task if you have access to later versions of .NET – Charleh Oct 02 '13 at 11:27
  • 3
    The code of `HandleIt` will be executed on the same thread that invoke the event not the thread you created. – Alessandro D'Andria Oct 02 '13 at 11:28
  • Running a thread to handle (all?) events is useless. The GUI needs its own events anyway. – H H Oct 02 '13 at 11:54
  • Ah, I hadn't actually read what Application.DoEvents did exactly, I just saw it in an example where someone was trying to do something similar and assumed it did what I thought it did... Now I see it makes no sense to call it here. Should I just use Thread.Sleep(10) or something instead? – moggizx Oct 02 '13 at 12:09
  • I now have a Task that executes the same code as ProcessEvents does except the while loop looks like this: while (!token.IsCancellationRequested) { Thread.Sleep(10); }. I can start and stop the thread perfectly and it cleans up just as I want,b ut no events are handled now! :( – moggizx Oct 02 '13 at 13:54

2 Answers2

6

There are many things wrong with this code. First off, it suffers from the illusion that the event handler runs on that worker thread. That's not how events work, the handler runs on the exact same thread as the code that fires the event. There is no mechanism in .NET to let it hop to another thread.

Something you can see with the debugger's Debug + Windows + Threads window. Set a breakpoint on the Init() method and one on the event handler. When they break, switch to that debugger screen and note the thread that's selected.

Using Application.DoEvents() is very dangerous. But not here, the thread hasn't created any windows so there are no events to do. Instead, the thread will just burn 100% core and not accomplish anything at all.

The mKeepHandlingIt variable does not in general do what you hope it does. When you run the Release build of your code on a 32-bit machine then thread will never see it being set to false and the thread will thus never exit. And Thread.Join() will deadlock. This is caused by the jitter optimizer, it stores the bool variable in a cpu register and will never reload it from memory. You must declare the variable volatile to prevent this optimization from being made. Which is a hack, you should always use a proper synchronization object to signal threads, an AutoResetEvent is appropriate here.

Just delete this code, it doesn't help you.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Well, my original problem is this: The call to Init is made from a BackgroundWorker that loads the view, which is in turn started by the main UI thread, but when the BackgroundWorker finishes, there is nothing left to listen for the events... If I subscribe to the events in the BackgroundWorker thread and then start a new thread that just loops with Thread.Sleep(1) or something like that, and then stop the thread and unsubscribe from the events in a third thread later, will that work? Or will the thread that listens for the events have to subscribe and unsubscribe to them itself? – moggizx Oct 02 '13 at 12:03
  • You haven't yet gotten to the conclusion that the event handler doesn't actually run on the thread. You can't get there unless you use the debugging hints I gave you. – Hans Passant Oct 02 '13 at 12:13
  • Well, it's quite irrelevant for me on which thread the code actually runs on in the end (my handler has to use Dispatcher.Invoke to execute its code anyway), I just want it TO run. I understand what you're saying, that the thread that raises the event also executes the handler code. The thing I'm confused about is: Does it matter on which thread I subscribe/unsubscribe to the event? And if not, why does the thread need to be alive in order for the handler to trigger? I'm pretty new to all this threading business still... as you may have realized by now. – moggizx Oct 02 '13 at 12:35
  • 1
    No, that doesn't matter. You are asking questions about code I cannot see, it is entirely unclear what code actually fires the event. Use the debugger to get insight. – Hans Passant Oct 02 '13 at 12:42
  • The event is fired by a third party library that I do not have the source code for... but, are you saying I actually don't need this thread at all? Should I be able to subscribe to the events in the BackgroundWorker and then when the BackgroundWorker is finished and dies, the events should still be handled by the handler method in the thread that fire the events? – moggizx Oct 02 '13 at 13:45
  • Okay, I have narrowed it down to the third party lib that I'm using being extremely unstable... no matter which approach I use it sometimes just fails and sometimes doesn't for no apparent reason. This answer did help me a lot so I'll accept it. – moggizx Oct 02 '13 at 15:29
2

For most cases you should never use doevents. s.

For example see:

Creating Threads with new Thread can run into trouble if no more threads were aviable. Therefore you could use the Threadpool.

But I would stronlgy recomment to have a look at Tasks and the TPL

Then you can start with

Task.Factory.StartNew()

There als awaitable when usinc the async libraries and you have functions like continuewith

see also MSDN await one ore more tasks

            // Wait for all tasks to complete.
            Task[] tasks = new Task[10];
            for (int i = 0; i < 10; i++)
            {
                tasks[i] = Task.Factory.StartNew(() => DoSomeWork(10000000));
            }
            Task.WaitAll(tasks);

Async and await also don't necessarily use threads but work asynchronous for this see: async-await-vs-threads

This is can be a great benefit to safe ressources. Especially when you have io operations.

Also a more complex way is to implement the listenings in a seperate process with some kind of servicebuses (what ever you want to achieve)

Community
  • 1
  • 1
Boas Enkler
  • 12,264
  • 16
  • 69
  • 143