Search code examples
c#multithreadingthreadpoolmanualreseteventcountdownevent

Using CountdownEvent and ManualResetEvent to control threads in ThreadPool


I've got the following multithreaded code excerpt that I've been working on to compare files following a zipped copy and unzip. The application is zipping a folder containing a variable number of files of various sizes, copying the files to a server, and unzipping them. Then the files are compared and this comparison is threaded out to a ThreadPool.

Here is the current full method:

public void FolderMoverLogic(string folderPathToZip, string unzipOutputDir)
{
    string folderRootDir = Path.GetDirectoryName(folderPathToZip);
    string folderNameToZip = Path.GetFileName(folderPathToZip);

    try
    {
        //Zips files in <folderPathToZip> into folder <zippedLocal>
        TransferMethods.CreateZipExternal(folderPathToZip, zippedlocal);
        //Copies zipped folder to server location
        File.Copy(zippedlocal + "\\" + folderNameToZip + ".zip", zippedserver + "\\" + folderNameToZip + ".zip");
        //Unzips files to final server directory
        TransferMethods.UnZip(zippedserver + "\\" + folderNameToZip + ".zip", unzipOutputDir + "\\" + folderNameToZip, sizeof(Int32));

        TransferMethods m = new TransferMethods();

        //Enumerate Files for MD5 Hash Comparison
        var files = from file in Directory.EnumerateFiles(folderPathToZip, "*", SearchOption.AllDirectories)
                    select new
                    {
                        File = file,
                    };

        int fileCount = 0;
        CountdownEvent countdown = new CountdownEvent(10000); 
        using (ManualResetEvent resetEvent = new ManualResetEvent(false))
        {
            foreach (var f in files)
            {
                Interlocked.Increment(ref fileCount);
                countdown.Reset(fileCount);
                try
                {
                    ThreadPool.QueueUserWorkItem(
                        new WaitCallback(c => 
                            {
                                //Check if any of the hashes have been different and stop all threads for a reattempt
                                if (m.isFolderDifferent)
                                {
                                    resetEvent.Set();
                                    CancellationTokenSource cts = new CancellationTokenSource();
                                    cts.Cancel(); // cancels the CancellationTokenSource 
                                    try
                                    {
                                        countdown.Wait(cts.Token);
                                    }
                                    catch (OperationCanceledException)
                                    {
                                        Console.WriteLine("cde.Wait(preCanceledToken) threw OCE, as expected");
                                    }
                                    return;
                                }
                                else
                                {
                                    //Sets m.isFolderDifferent to true if any files fail MD5 comparison
                                    m.CompareFiles(f.File, folderRootDir, unzipOutputDir);
                                }
                                if (Interlocked.Decrement(ref fileCount) == 0)
                                {
                                    resetEvent.Set();
                                }
                                countdown.Signal();
                            }));

                }
                catch (Exception ex)
                {
                    Console.WriteLine(ex.ToString());
                }
            }
            countdown.Wait();
            resetEvent.WaitOne();
            resetEvent.Close();





        }
    }
    catch (Exception Ex)
    {
        Console.WriteLine(Ex.Message);
    }
}

Useful resources looked at so far:

Is it safe to signal and immediately close a ManualResetEvent?

Stopping all thread in .NET ThreadPool?

MSDN CountdownEvent

ThreadPool Logic Requirements:

  • Compare all enumerated files locally and on the server
  • Return from all threads if hashing does not match

Previous ThreadPool Code:

using (ManualResetEvent resetEvent = new ManualResetEvent(false))
{
    foreach (var f in files)
    {
        testCount++;
        try
        {
            //Thread t = new Thread(() => m.CompareFiles(f.File, unzipped, orglsource));
            //t.Start();
            //localThreads.Add(t);
            ThreadPool.QueueUserWorkItem(
                new WaitCallback(c => 
                    {
                        if (resetEvent.WaitOne(0))  //Here is the `ObjectDisposedException`
                        {
                            return;
                        }
                        if (!m.Folderdifferent)
                        {
                            m.CompareFiles(f.File, folderRootDir, unzipOutput);
                        }
                        else
                        {
                            resetEvent.Set();
                        }
                        if (Interlocked.Decrement(ref fileCountZipped) == 0)
                        {
                            resetEvent.Set();
                        }

                    }));

        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.ToString());
        }

    }
    resetEvent.WaitOne();
}

I was getting ObjectDisposedExceptions periodically with the previous code shown.

My questions are as such:

  1. Is the current method thread-safe?
  2. Is the logic sound?
  3. Any improvement ideas for performance or thread safety
  4. Does the current method I have at the top resolves the previous code exceptions

I've been testing this code and it has been working without exceptions but am looking at some more experienced feedback.


Solution

  • some notes:

    • wasn't supposed to be like this?:
      CountdownEvent countdown = new CountdownEvent(files.Count()); 
    • Is it safe? - NO - I simply don't like the idea with CountdownEvent, if ANY operation with ANY file fails you don't get signal and application hangs on countdown.Wait(), I prefer using TPL Tasks instead - and instead of countdown.Wait() and use Task.WaitAll(tasks)
    • never use direct "foreach variable" in threads (this thread explains why), so instead of:

      foreach (var f in files)
      {
          Task.Run(() =>
          {
               var whateveryDoWithIt = f.File; 
          }
      }
      do this:
      foreach (var f in files)
      {
          var ftemp = f;
          Task.Run(() =>
          {
               var whateveryDoWithIt = ftemp.File; 
          }
      }

    • to answer if it's thread-safe I would answer: Yes if you fix the points above and all methods used in it are also thread-safe