views:

213

answers:

8

I am writing a GUI application.

The application is opening multiple threads during it's life time. One of the threads is handling events that can come from other applications, so it is waiting in a while(true) loop for the event which is never been terminated.

The user can close the application in any minute. I want to close all the threads that the main application had opened.

I am using Process.GetCurrentProcess().Kill(); to deal with this problem at the moment.

Is this a good solution? If not, why and what is the proper way to deal with this problem, how to close all threads that were opened by the main application?

+16  A: 

If you create the new threads as background threads (by setting IsBackground before starting them), they will automatically stop when the main thread (the application thread) terminates.

(From MSDN):

A thread is either a background thread or a foreground thread. Background threads are identical to foreground threads, except that background threads do not prevent a process from terminating. Once all foreground threads belonging to a process have terminated, the common language runtime ends the process. Any remaining background threads are stopped and do not complete.

adrianbanks
Setting `IsBackground = true` sounds like it could be appropriate for your case. I've had to use that property when I had a thread that was continuously blocking on `Console.ReadLine` and waiting for input. *However,* in most cases, you should try to design your program so that it doesn't allow closing until after calculation or processing threads are complete, or you could build a cancellation mechanism into the calculation or processing code.
jnylen
I agree with @jnylen, you want to have some sort of mechanism to let your threads exit gracefully, not just get shutdown while possibly in the middle of some operation.
Coding Gorilla
@jnylen: Yes, if the threads are doing something that **requires** clean termination (eg. writing to a file), you need clean termination by signalling them to stop and then waiting. If the threads can be interrupted, setting them to a be background thread will suffice.
adrianbanks
+3  A: 

Once you already have threads waiting for some events, just add one more event that when triggered will instruct the thread to terminate.

In case you don't need to provide some means of graceful shutdown for other threads, you can switch them into the “background thread” mode to ensure automatic termination — see MSDN for a thorough discussion of this topic.

Ondrej Tucny
+1  A: 

There are a lot of ways to deal with this, but ideally you want your threads to exit normally on their own rather than just killing the process.

You could do something very simple like this:

public class ThreadSignal
{
   public bool Stop { get; set; }
}

Then in your thread loop, do:

public void DoWork(object state) 
{
   ThreadSignal signal = (ThreadSignal)state;
   while(!signal.Stop)
   {
      // Do work here    
   }
}

Then when you're ready to stop, set your ThreadSignal.Stop to true. This is a very simple example, but it gives you a starting point.

Coding Gorilla
A: 

As you mentioned it's a GUI application so the main thread which is responsible for message loop is responsible for alerting the infinite (while(true)) loop that user wants to exit the program. I recommend to replace true with another boolean for signaling that user has closed the window like this: while(windowIsOpen) and set it to false on the unload of your form.

Xaqron
+1  A: 

You should wait in the loop with a ManualResetEvent (or AutoResetEvent). Then just set a member variable to true when you are shutting down:

public class MyForm : Form
{
    private AutoResetEvent _workTrigger = new AutoResetEvent();
    private bool _shuttingDown = false;
    private Thread _thread;

    public void Form_Initialize()
    {
        _thread = new Thread(MyThreadMethod);
        _thread.Start();
    }

    public static void MyThreadMethod(object State)
    {
        while (!_shuttingDown)
        {
            //wait for jobs.
            _workTrigger.WaitOne(); //can add a timeout as parameter.

            //do some work here

        }

    }


    public void Form_Closing(object source, EventArgs e)
    {
       _shuttingDown = true;
       _workTrigger.Set();

       //wait for it to exit. You could use the timeout
       //parameter and a loop to not block the UI
       _thread.Join();  
    }
}
jgauffin
A: 

Don't lose your threads around the application - keep'em somewhere (List<Thread> will do fine). Then when the time is right (closing time) notify each one that it should finish what it's doing and exit.

Then, .Join() all of them, then allow application to exit.

Don't ever go to 'ThreadAbort' realm, it's dark side of the force that lurks there.

Daniel Mošmondor
A: 

Generally how I do this is:

  • Create a Class that encapsulates this behavior (e.g. handling incoming messages in the background
  • Have the Class inherit from IDisposable. When Dispose() is called set a private variable named _disposed
  • Create my dedicated thread in my Class constructor.
  • Have a private AutoResetEvent named _workToDo. Your background thread will wait on this event and only do a work loop when this event is signaled.
  • Have a public method to send the message to your background worker that queues the work up and then sets _workToDo to tell your background thread to do the work.

Putting this all together, you get:

public class BackgroundProcessor : IDisposed
{
  private Thread _backgroundThread;
  private bool _disposed;
  private AutoResetEvent _workToDo = new AutoResetEvent(false);
  // where T is a class with the set of parameters for your background work
  private Queue<T> _workQueue = Queue.Synchronized(new Queue<T>);

  public BackgroundProcessor()
  {
    _backgroundThread = new Thread(DoBackgroundWork);
    _backgroundThread.Start();
  }

  public void Dispose()
  {
    _disposed = true;

    // Wait 5 seconds for the processing of any previously submitted work to finish.
    // This gives you a clean exit.  May want to check return value for timeout and log
    // a warning if pending background work was not completed in time.
    // If you're not sure what you want to do yet, a Debug.Assert is a great place to
    // start because it will let you know if you do or don't go over time in general
    // in your debug builds.
    // Do *not* Join() and wait infinitely.  This is a great way to introduce shutdown
    // hangs into your app where your UI disappears but your process hangs around
    // invisibly forever.  Nasty problem to debug later...
    Debug.Assert(_backgroundThread.Join(5000)); 
  }

  // Called by your 'other application'
  public void GiveMeWorkToDo(T workParameters)
  {
    _workQueue.Enqueue(workParameters);
    _workToDo.Set();
  }

  private void DoBackgroundWork()
  {
    while (!_disposed)
    {
      // 500 ms timeout to WaitOne allows your Dispose event to be detected if there is
      // No work being submitted.  This is a fancier version of a Thread.Sleep(500)
      // loop.  This is better because you will immediately start work when a new
      // message is posted instead of waiting for the current Sleep statement to time
      // out first.
      _workToDo.WaitOne(500);

      // It's possible multiple sets of work accumulated or that the previous loop picked up the work and there's none left.  This is a thread safe way of handling this.
      T workParamters = _workQueue.Count > 0 ? workParameters = _workQueue.Dequeue() : null;
      do
      {
        DoSomething(workParameters);

        workParameters = _workQueue.Count > 0 ? workParameters = _workQueue.Dequeue() : null;
      } while (workParameters != null)
    }
  }
}
Jay
A: 

Consider using the BackGroundWorker class. Since it's using the threadpool (via BeginInvoke()), you'd get background threads. As a bonus you get convenient progress reporting, cancellation and completion callbacks (already marshalled to the UI thread).

ohadsc