Thread.Interrupt to stop long sleep at app shutdown - Is there a better approach

Question

I'm having a small background thread which runs for the applications lifetime - however when the application is shutdown, the thread should exit gracefully.

The problem is that the thread runs some code at an interval of 15 minutes - which means it sleeps ALOT.

Now in order to get it out of sleep, I toss an interrupt at it - my question is however, if there's a better approach to this, since interrupts generate ThreadInterruptedException.

Here's the gist of my code (somewhat pseudo):

public class BackgroundUpdater : IDisposable
{
    private Thread myThread;
    private const int intervalTime = 900000; // 15 minutes
    public void Dispose()
    {
        myThread.Interrupt();
    }

    public void Start()
    {
        myThread = new Thread(ThreadedWork);
        myThread.IsBackground = true; // To ensure against app waiting for thread to exit
        myThread.Priority = ThreadPriority.BelowNormal;
        myThread.Start();
    }

    private void ThreadedWork()
    {
        try
        {
            while (true)
            {
                Thread.Sleep(900000); // 15 minutes
                DoWork();
            }
        }
        catch (ThreadInterruptedException)
        {
        }
    }
}

Answer 1

There's absolutely a better way - either use Monitor.Wait/Pulse instead of Sleep/Interrupt, or use an Auto/ManualResetEvent. (You'd probably want a ManualResetEvent in this case.)

Personally I'm a Wait/Pulse fan, probably due to it being like Java's wait()/notify() mechanism. However, there are definitely times where reset events are more useful.

Your code would look something like this:

private readonly object padlock = new object();
private volatile bool stopping = false;

public void Stop() // Could make this Dispose if you want
{
    stopping = true;
    lock (padlock)
    {
        Monitor.Pulse(padlock);
    }
}

private void ThreadedWork()
{
    while (!stopping)
    {
        DoWork();
        lock (padlock)
        {
            Monitor.Wait(padlock, TimeSpan.FromMinutes(15));
        }
    }
}

For more details, see my threading tutorial, in particular the pages on deadlocks, waiting and pulsing, the page on wait handles. Joe Albahari also has a tutorial which covers the same topics and compares them.

I haven't looked in detail yet, but I wouldn't be surprised if Parallel Extensions also had some functionality to make this easier.

Answer 2

You could use an Event to Check if the Process should end like this:

var eventX = new AutoResetEvent(false);
while (true)
{
    if(eventX.WaitOne(900000, false))
    {
        break;
    }
    DoWork();
}

Answer 3

There is CancellationTokenSource class in .NET 4 and later which simplifies this task a bit.

private readonly CancellationTokenSource cancellationTokenSource = 
    new CancellationTokenSource();

private void Run()
{
    while (!cancellationTokenSource.IsCancellationRequested)
    {
        DoWork();
        cancellationTokenSource.Token.WaitHandle.WaitOne(
            TimeSpan.FromMinutes(15));
    }
}

public void Stop()
{
    cancellationTokenSource.Cancel();
}

Don't forget that CancellationTokenSource is disposable, so make sure you dispose it properly.

Answer 4

One method might be to add a cancel event or delegate that the thread will subscribe to. When the cancel event is invoke, the thread can stop itself.

Answer 5

I absolutely like Jon Skeets answer. However, this might be a bit easier to understand and should also work:

public class BackgroundTask : IDisposable
{
    private readonly CancellationTokenSource cancellationTokenSource;
    private bool stop;

    public BackgroundTask()
    {
        this.cancellationTokenSource = new CancellationTokenSource();
        this.stop = false;
    }

    public void Stop()
    {
        this.stop = true;
        this.cancellationTokenSource.Cancel();
    }

    public void Dispose()
    {
        this.cancellationTokenSource.Dispose();
    }

    private void ThreadedWork(object state)
    {
        using (var syncHandle = new ManualResetEventSlim())
        {
            while (!this.stop)
            {
                syncHandle.Wait(TimeSpan.FromMinutes(15), this.cancellationTokenSource.Token);
                if (!this.cancellationTokenSource.IsCancellationRequested)
                {
                    // DoWork();
                }
            }
        }
    }
}

Or, including waiting for the background task to actually have stopped (in this case, Dispose must be invoked by other thread than the one the background thread is running on, and of course this is not perfect code, it requires the worker thread to actually have started):

using System;
using System.Threading;

public class BackgroundTask : IDisposable
{
    private readonly ManualResetEventSlim threadedWorkEndSyncHandle;
    private readonly CancellationTokenSource cancellationTokenSource;
    private bool stop;

    public BackgroundTask()
    {
        this.threadedWorkEndSyncHandle = new ManualResetEventSlim();
        this.cancellationTokenSource = new CancellationTokenSource();
        this.stop = false;
    }

    public void Dispose()
    {
        this.stop = true;
        this.cancellationTokenSource.Cancel();
        this.threadedWorkEndSyncHandle.Wait();
        this.cancellationTokenSource.Dispose();
        this.threadedWorkEndSyncHandle.Dispose();
    }

    private void ThreadedWork(object state)
    {
        try
        {
            using (var syncHandle = new ManualResetEventSlim())
            {
                while (!this.stop)
                {
                    syncHandle.Wait(TimeSpan.FromMinutes(15), this.cancellationTokenSource.Token);
                    if (!this.cancellationTokenSource.IsCancellationRequested)
                    {
                        // DoWork();
                    }
                }
            }
        }
        finally
        {
            this.threadedWorkEndSyncHandle.Set();
        }
    }
}

If you see any flaws and disadvantages over Jon Skeets solution i'd like to hear them as i always enjoy learning ;-) I guess this is slower and uses more memory and should thus not be used in a large scale and short timeframe. Any other?