BackgroundWorker and ConcurrentQueue

Question

I have a FileSystemWatcher which is looking for new files, putting the filenames in a Queue. In a seperate Thread the Queue is worked off. My code is working, but I question if there could be information lost, because of the asynchronous process. Please watch the code explained by comments: (I think maybe I need something like a thread lock somewhere?) (Code is simplified)

public class FileOperatorAsync
{
  private ConcurrentQueue<string> fileQueue;
  private BackgroundWorker worker;
  private string inputPath;

  public FileOperatorAsync(string inputPath)
  {
     this.inputPath = inputPath;
     fileQueue = new ConcurrentQueue<string>();
     worker = new BackgroundWorker();
     worker.WorkerSupportsCancellation = true;
     worker.DoWork += worker_DoWork;
     Start();
  }

  void worker_DoWork(object sender, DoWorkEventArgs e)
  {
     try
     {
        string file;
        while (!worker.CancellationPending && fileQueue.TryDequeue(out file)) //As long as queue has files
        {
          //Do hard work with file
        }
        //Thread lock here?
        //If now Filenames get queued (Method Execute -> Worker is still busy), they wont get recognized.. or?
     }
     catch (Exception ex)
     {
        //Logging
     }
     finally
     {
        e.Cancel = true;
     }
  }

  public void Execute(string file) //called by the FileSystemWatcher
  {
     fileQueue.Enqueue(file);
     Start(); //Start only if worker is not busy
  }

  public void Start()
  {
     if (!worker.IsBusy)
        worker.RunWorkerAsync();
  }

  public void Stop()
  {
     worker.CancelAsync();
  }

}

Answer 1

Yes, you may have a problem with Execute. It can leave a file unprocessed by your worker.

You can solve it in two ways:
1) Your worker doesn't finish after processing all queued files. It waits on a AutoResetEvent for next file to process. In that case Execute should notify the worker by calling AutoResetEvent.Set.
Example:

AutoResetEvent event;
...
// in worker_DoWork
while(!worker.CancellationPending){
    event.WaitOne();
    // Dequeue and process all queued files
}

...
// in Execute
fileQueue.Enqueue(file);
event.Set();

2) Your worker finishes after processing all queued files (as you do now) but you can check in BackgroundWorker.RunWorkerCompleted whether there are still files to process and run the worker once again.
In that case if Execute hasn't started the worker because it was busy then the worker will be started again in BackgroundWorker.RunWorkerCompleted and the pending file will be processed.

// in  worker_RunWorkerCompleted
if (!fileQueue.IsEmpty())
    Start();

Note: if you decide to use BackgroundWorker.RunWorkerCompleted in a non-GUI application then you should be careful in Start because BackgroundWorker.RunWorkerCompleted can be invoked not on the thread where you call Execute and a race condition will occur in Start. More info: BackgroundWorker.RunWorkerCompleted and threading

if you call Start() from two different threads simultaneously then they both can see that worker.IsBusy == false and they both will call worker.RunWorkerAsync(). The thread which calls worker.RunWorkerAsync() a little later than another thread will throw an InvalidOperationException. So you should catch that exception or wrap IsBusy+RunWorkerAsync into a critical section with a lock to avoid the race condition and the exception throwing.

Answer 2

To not to have to worry about the problem, when the queue is empty and Start is called before the worker exits, you can try not leaving the worker method at all:

while (!worker.CancellationPending)
{
    while (!worker.CancellationPending && !fileQueue.TryDequeue(out file))
    {
        Thread.Sleep(2000);
    }

    if (worker.CancellationPending)
    {
        break;
    }
    //
}

Other possibility, without the inelegant sleeping would be to use the ManualResetEvent class to signal when the queue is empty and stops being empty.