0

I'm trying to create an application in Net 5 that watches a folder and any time files are being dropped in the folder, it should run a certain set of tasks (getting some info from the files, copy them to new location, among others). I thought I could implement both Net's FileSystemWatcher and concurrent collections (from the Collections.Concurrent namespace here, but I run into the problem that I cannot run it with being async.

I initialize the watcher like this (following the docs of MS):

public BlockingCollection<string> fileNames = new BlockingCollection<string>();
public void InitiateWatcher()
{
    using FileSystemWatcher watcher = new FileSystemWatcher(@"C:\temp"); //test dir

    watcher.NotifyFilter = NotifyFilters.Attributes
             | NotifyFilters.CreationTime
             | NotifyFilters.DirectoryName
             | NotifyFilters.FileName

    watcher.Created += OnCreated;

    watcher.Filter = "*.*";
    watcher.IncludeSubdirectories = true;
    watcher.EnableRaisingEvents = true;

    Console.WriteLine("Press e to exit.");
    Console.ReadLine();
}

private void OnCreated(object sender, FileSystemEventArgs e)
{
    string value = $"Created: {e.FullPath}";
    // this of course creates problems, since it is a async task running in sync mode.
    await PipelineStages.ReadFilenamesAsync(_sourcePath, fileNames);
    // do other work with the result from ReadFilenameAsync
    Console.WriteLine(value);
}

My PipelineStages class, which does most of the real work with the files, looks like this:

public static class PipelineStages
{
    public static Task ReadFilenamesAsync(string path, BlockingCollection<string> output)
    {
        return Task.Factory.StartNew(() =>
        {
            foreach (string fileName in Directory.EnumerateFiles(path, "*.*", SearchOption.AllDirectories))
            {
                output.Add(fileName);
            }
            output.CompleteAdding();
        }, TaskCreationOptions.LongRunning);
    }
}

If I turn the OnCreated method async it throws error that the return type is not valid. That kinda makes sense to me, although I don't know the way forward on this.

I see two errors: One error is when the code hits the output.add(fileName) line, it throws an System.InvalidOperationException: 'The collection has been marked as complete with regards to additions.'

The other one is that I notice in the onCreated method that filenames get written to the console that are in very different folders, seemingly randomly.

So, basically, I have two questions:

  • Is this the right approach? (If not, what are other alternatives)
  • If this is the right approach, how can I fix these problems?
CMorgan
  • 645
  • 2
  • 11
  • 33
  • 3
    `FileSystemWatcher` handlers should do as little as possible and return as quickly as possible. A better approach IMO is to dump off the notification to another thread. I personally use an instance of the `System.Threading.Channels.Channel` class. Each one of my `FileSystemWatcher` event-handlers takes the event data, writes it to Channel, and returns. Meanwhile my main thread has a one-time await-function that continuously monitors the channel (with `await foreach`) and prossesses each notification. – Joe Mar 08 '22 at 20:38
  • 2
    @Martas `async void ` is generally discouraged. – Neil Mar 08 '22 at 20:51
  • @Neil that was my understanding too. – CMorgan Mar 08 '22 at 20:52
  • 1
    Does this answer your question? [Should I avoid 'async void' event handlers?](https://stackoverflow.com/questions/19415646/should-i-avoid-async-void-event-handlers) – tymtam Mar 09 '22 at 00:44
  • Have you tried making the event handler asynchronous, by adding the `async` keyword? `private async void OnCreated(object sender, FileSystemEventArgs e)` What exactly is the compile error that you got? – Theodor Zoulias Mar 09 '22 at 01:53
  • @TheodorZoulias there is no compile error. The error is when running the code: When trying to add the filename to the BlockingCollection output it throws an error System.InvalidOperationException: 'The collection has been marked as complete with regards to additions.' Updated my post. – CMorgan Mar 09 '22 at 17:04
  • 1
    Have you tried removing the `output.CompleteAdding();` line? – Theodor Zoulias Mar 09 '22 at 21:12

1 Answers1

2

Firstly, it looks like you prematurely call output.CompleteAdding(), so every subsequent execution of PipelineStages.ReadFilenamesAsync gets the foregoing System.InvalidOperationException error. Secondly, while async void is indeed generally discouraged, it is admissible in the following cases:

  • event handlers
  • the Main method

BUT as long as such an async void event handler is basically a fire‑and‑forget function, you should make it bulletproof with regard to any captured variables whose state can be invalidated in the outer scope (disposed, CompleteAdding, and the like).
I have rigged up the following async‑contrived short self‑contained example to demonstrate the point I am trying to make:

using System.Collections.Concurrent;
using System.Diagnostics;

object lockObj = new();
using var watcher = new FileSystemWatcher(@".");
using BlockingCollection<string?> queue = new();

async void FileCreatedHandlerAsync(object _, FileSystemEventArgs e) => await Task.Run(() => {
  var lockTaken = false;
  try
  {
    Monitor.Enter(lockObj, ref lockTaken);
    if (!queue.IsCompleted)
      queue.Add(e.Name);
  }
  catch (ObjectDisposedException) { }
  finally {
    if (lockTaken)
      Monitor.Exit(lockObj);
  }
});
watcher.Created += FileCreatedHandlerAsync;
watcher.EnableRaisingEvents = true;

var consumer = Task.Run(async () => {
  foreach (string? name in queue.GetConsumingEnumerable())
    await Task.Run(() => Console.WriteLine($"File has been created: \"{name}\"."));
  
  Debug.Assert(queue.IsCompleted);
});

Console.WriteLine("Press any key to exit.");
Console.ReadLine();

watcher.Created -= FileCreatedHandlerAsync;
lock (lockObj) {
  queue.CompleteAdding();
}
bool completed = consumer.Wait(100);
Debug.Assert(completed && TaskStatus.RanToCompletion == consumer.Status);