Two Thread tasks in Winforms Continuously Checking Back

Question

I am currently trying to keep a counter on C# on a local file folder for new files that are created.

I have two sub directories to CD and LP that I have to keep checking. With counters that makes sure that the count of folders made have not exceeded the count set by the user.

public static int LPmax { get; set; }
public static int CDmax { get; set; }

public static int LPcounter2 { get; set; }
public static int CDcounter2 { get; set; } 

public static int LPCreated;
public static int CDCreated;
public static int oldLPCreated;
public static int oldCDCreated;

FileSystemWatcher CDdirWatcher = new FileSystemWatcher();
FileSystemWatcher LPdirWatcher = new   FileSystemWatcher();

//watch method should run in the background as checker
public void watch()
{ 
        CDdirWatcher.Path = @"C:\Data\LotData\CD";
        CDdirWatcher.Filter = "EM*";
        CDdirWatcher.NotifyFilter = NotifyFilters.DirectoryName | NotifyFilters.LastWrite;
        CDdirWatcher.EnableRaisingEvents = true;
        CDdirWatcher.Created += CDdirWatcher_Created;


        LPdirWatcher.Path = @"C:\Data\LotData\LP";
        LPdirWatcher.Filter = "EM*";
        LPdirWatcher.NotifyFilter = NotifyFilters.DirectoryName;
        LPdirWatcher.EnableRaisingEvents = true;
        LPdirWatcher.Created += LPdirWatcher_Created;
        Thread.Sleep(10); 
}
private static void CDdirWatcher_Created(object sender, FileSystemEventArgs e)
{ 
    CDCreated += 1;
}
private static void LPdirWatcher_Created(object sender, FileSystemEventArgs e)
{ 
    LPCreated += 1;
}

The above method works fine, and the criteria is that it has to be less count then the one set

public void checker()
{
    if(CDCreated>CDmax)
    {
        popupbx();
    }
    if(LPCreated>LPmax)
    {
        popupbx();
    }
}

The problem is my main method where I have two threads which need to continuously check the two criteria to see if the counter has been exceeded.

public Form1()
{
    InitializeComponent();

    //Implementing Threads asynchronously
    Thread oThreadone = new Thread(() =>
    {
        //Do what u wanna……
        watch();
    });

    Thread oThreadtwo = new Thread(() =>
    {
        //Do what u wanna……
        checker();
    });

    //Calling thread workers
    oThreadone.Start(); 
    oThreadone.IsBackground = true; 
    oThreadtwo.Start();
    oThreadtwo.IsBackground = true;
}

Mkdir fires the counters in debug mode, but thread two doesn't check for the counters after they fire.

Answer 1

The first major thing wrong with your code is that neither of threads you create are needed, nor do they do what you want. Specifically, the FileSystemWatcher object itself is already asynchronous, so you can create it in the main thread. In fact, you should, because there you could set FileSystemWatcher.SynchronizingObject to the current form instance so that it will raise its events in that object's synchronization context. I.e. your event handlers will be executed in the main thread, which is what you want.

So the first method, watch(), rather than being executed in a thread, just call it directly.

Which brings me to the second method, checker(). Your method for the thread doesn't loop, so it will execute the two tests, and then promptly exit. That's the end of that thread. It won't stay around long enough to monitor the counts as they are updated.

You could fix it by looping in the checker() method, so that it never exits. But then you run into problems of excessive CPU usage. Which you'll fix by adding sleep statements. Which then you're wasting a thread most of the time. Which you could fix by using async/await (e.g. await Task.Delay()). Except that would just unnecessarily complicate the code.

Instead, you should just perform each check after each count is updated. In theory, you could just display the message immediately. But that will block the event handler subscribed to FileSystemWatcher, possibly delaying additional reports. So you may instead prefer to use Control.BeginInvoke() to defer display of the message until after the event handler has returned.

So, taking all that into account, your code might instead look more like this:

public Form1()
{
    InitializeComponent();

    watch();
}

private void CDdirWatcher_Created(object sender, FileSystemEventArgs e)
{ 
    CDCreated += 1;
    if (CDCreated > CDmax)
    {
        BeginInvoke((MethodInvoker)popupbx);
    }
}

private static void LPdirWatcher_Created(object sender, FileSystemEventArgs e)
{ 
    LPCreated += 1;
    if (LPCreated > LPmax)
    {
        BeginInvoke((MethodInvoker)popupbx);
    }
}

You can remove the checker() method altogether. The watch() method can remain as it is, though I would change the order of subscribing to the Created event and the assignment of EnableRaisingEvents. I.e. don't enable raising events until you've already subscribed to the event. The FileSystemWatcher is unreliable enough as it is, without you giving it a chance to raise an event before you're ready to observe it. :)

This is based on your current implementation. Note though that if files keep getting created, the message will be displayed over and over. If they are created fast enough, new messages will be displayed before the user can dismiss the previously-displayed one(s). You may prefer to modify your code to prevent this. E.g. unsubscribe from the event after you've already exceeded the max count, or at least temporarily inhibit the display of the message while one such message is already being displayed. Exactly what to do is up to you, and beyond the scope of your question and thus the scope of this answer.