.NET BackgroundWorker RunWorkerAsync() oddly gets called twice

Question

CODE UPDATED TO REFLECT ANSWER: SAME PROBLEM STILL OCCURS

This class is supposed to run all tasks in the list, sleep and then wake up and repeat the process infinitely. For some reason though, after the first sleep, the sleepThread.RunWorkerAsync() call gets called twice for some reason. I can obviously solve this by:

if (!sleepThread.IsBusy) { sleepThread.RunWorkerAsync(); }

but that feels like a work around.

Here is the main routine class:

public class ServiceRoutine
{
    private static volatile ServiceRoutine instance;
    private static object instanceLock = new object();
    private static object listLock = new object();
    private static readonly List<Task> taskList = new List<Task>()
    {
        new UpdateWaferQueueTask(),
        new UpdateCommentsTask(),
        new UpdateFromTestDataTask(),
        new UpdateFromTestStationLogsTask(),
        new UpdateFromWatchmanLogsTask(),
        new UpdateStationsStatusTask()
    };

    private List<Task> runningTasks;
    private BackgroundWorker sleepThread;
    private Logger log;

    private ServiceRoutine()
    {
        log = new Logger();
        runningTasks = new List<Task>();

        sleepThread = new BackgroundWorker();
        sleepThread.WorkerReportsProgress = false;
        sleepThread.WorkerSupportsCancellation = false;
        sleepThread.DoWork += (sender, e) =>
        {
            int sleepTime = ConfigReader.Instance.GetSleepTime();
            log.Log(Logger.LogType.Info, 
                "service sleeping for " + sleepTime / 1000 + " seconds");
            Thread.Sleep(sleepTime);
        };
        sleepThread.RunWorkerCompleted += (sender, e) => { Run(); };
    }
    public static ServiceRoutine Instance
    {
        get
        {
            if (instance == null)
            {
                lock (instanceLock)
                {
                    if (instance == null)
                    {
                        instance = new ServiceRoutine();
                    }
                }
            }
            return instance;
        }
    }

    public void Run()
    {
        foreach (Task task in taskList)
        {
            lock (listLock) 
            {
                runningTasks.Add(task);
                task.TaskComplete += (completedTask) =>
                {
                    runningTasks.Remove(completedTask);
                    if (runningTasks.Count <= 0)
                    {
                        sleepThread.RunWorkerAsync();
                    }
                };
                task.Execute();
            }
        }
    }
}

this is called like this:

ServiceRoutine.Instance.Run();

from the service start method. Here is the Task class as well:

public abstract class Task
{
    private Logger log;
    protected BackgroundWorker thread;

    public delegate void TaskPointer(Task task);
    public TaskPointer TaskComplete;

    public Task()
    {
        log = new Logger();
        thread = new BackgroundWorker();
        thread.WorkerReportsProgress = false;
        thread.DoWork += WorkLoad;
        thread.RunWorkerCompleted += FinalizeTask;
    }

    protected abstract string Name { get; }
    protected abstract void WorkLoad(object sender, DoWorkEventArgs e);

    private string GetInnerMostException(Exception ex)
    {
        string innerMostExceptionMessage = string.Empty;
        if (ex.InnerException == null) { innerMostExceptionMessage = ex.Message; }
        else
        {
            while (ex.InnerException != null)
            {
                innerMostExceptionMessage = ex.InnerException.Message;
            }
        }
        return innerMostExceptionMessage;
    }
    protected void FinalizeTask(object sender, RunWorkerCompletedEventArgs e)
    {
        try
        {
            if (e.Error != null)
            {
                string errorMessage = GetInnerMostException(e.Error);
                log.Log(Logger.LogType.Error, this.Name + " failed: " + errorMessage);
            }
            else
            {
                log.Log(Logger.LogType.Info, "command complete: " + this.Name);
            }
        }
        catch (Exception ex)
        {
            string errorMessage = GetInnerMostException(ex);
            log.Log(Logger.LogType.Error, this.Name + " failed: " + errorMessage);
        }
        finally { TaskComplete(this); }
    }
    public void Execute()
    {
        log.Log(Logger.LogType.Info, "starting: " + this.Name);
        thread.RunWorkerAsync();
    }
}

The question is, why is sleepThread.RunWorkerAsync() getting called twice and is there a better way to get this work without checking if the thread is busy before calling it?

Answer 1

SOLVED

I tried several different locking techniques on the runningTasks list but nothing worked. After changing runningTasks to a BlockingCollection, everything worked perfectly.

Here is the new add/remove implementation using a BlockingCollection instead of a List:

foreach (Task task in taskList)
{
    runningTasks.Add(task);
    task.TaskComplete += (completedTask) =>
    {
        runningTasks.TryTake(out completedTask);
        if (runningTasks.Count <= 0 && completedTask != null)
        {
            sleepThread.RunWorkerAsync();
        }
    };
    task.Execute();
}

Answer 2

You are facing a race condition here. The problem is in the TaskComplete callback. Last two tasks remove themselves from the runningTasks list before executing the if condition. When it is executed, the list count is zero. You should lock the list before changing its. The lock needs to be taken in the TaskComplete callback:

runningTasks.Add(task);
task.TaskComplete += (completedTask) =>
{
    lock (runningTasks) 
    {
        runningTasks.Remove(completedTask);
        if (runningTasks.Count <= 0)
        {
            sleepThread.RunWorkerAsync();
        }
    }
};
task.Execute();