Pergunta

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.

Foi útil?

Solução

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

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top