Second Task does not end although loop parameter condition is updated in separate task

Question

Method readAllFilesAtRootFolder:

public static Dictionary<string, List<mediaFile>> readAllFilesAtRootFolder(
    string rootFolder, string[] extensions, bool subFolders,
    ConcurrentQueue<mediaFile> fileQueue, CancellationToken cancellationToken)
{
    var allFiles = new Dictionary<string, List<mediaFile>>();
    IEnumerable<string> files;
    try
    {
        if (subFolders)
        {
            files = extensions.SelectMany(ext => System.IO.Directory.GetFiles(
                rootFolder, ext, SearchOption.AllDirectories));
        }
        else
        {
            files = extensions.SelectMany(ext => System.IO.Directory.GetFiles(
                rootFolder, ext, SearchOption.TopDirectoryOnly));
        }
        int i = 1;
        foreach (var file in files)
        {
            if (cancellationToken.IsCancellationRequested)
                break;

            mediaFile mediaFile = new mediaFile();
            string extension = Path.GetExtension(file).Replace(".", "").ToUpper();
            // If the extension is not empty, group the files by extension
            if (!string.IsNullOrEmpty(extension))
            {
                if (!allFiles.ContainsKey(extension))
                {
                    allFiles[extension] = new List<mediaFile>();
                }
                mediaFile.fileName = file;
                //mediaFile.exif_date = getImageExifDate(file.ToString());
                fileQueue.Enqueue(mediaFile);
                allFiles[extension].Add(mediaFile);
            }
        }
        return allFiles;
    }
    catch { }
    return allFiles;
}

Method readAllEXIFdates:

public static void readAllEXIFdates(ConcurrentQueue<mediaFile> fileQueue,
    ConcurrentDictionary<string, List<mediaFile>> resultDictionary,
    CancellationToken cancellationToken, bool producerCompleted)
{
    var allEXIFFiles = new Dictionary<string, List<mediaFile>>();

    {
        //Provide some free time for the main thread
        //Thread.Sleep(100);
        while (!cancellationToken.IsCancellationRequested
            && (!producerCompleted || !fileQueue.IsEmpty))
        {
            if (fileQueue.TryDequeue(out var mediaFile))
            {
                // Process EXIF date
                mediaFile.exif_date = getImageExifDate(mediaFile.fileName);
                var extension = Path.GetExtension(mediaFile.fileName)
                    .ToUpperInvariant().TrimStart('.');

                if (!string.IsNullOrEmpty(extension))
                {
                    resultDictionary.AddOrUpdate(extension,
                        new List<mediaFile> { mediaFile },
                        (key, existingList) =>
                        {
                            existingList.Add(mediaFile);
                            return existingList;
                        });
                }
            }
            else
            {
                // Thread.Sleep(100);
                // Wait briefly if no files are available in the queue
            }
        }
    }
}

Button click event handler:

private async void button4_Click_1(object sender, EventArgs e)
{
    string[] extenions = getSelectExtensions(chkExtensions);

    label5.Text = "Raading files, please wait....";

    _cancellationTokenSource = new CancellationTokenSource();

    string[] extensions = getSelectExtensions(chkExtensions);

    label5.Text = "Reading files, please wait...";

    // Define shared resources
    var fileQueue = new ConcurrentQueue<mediaFile>();
    var resultDictionary = new ConcurrentDictionary<string, List<mediaFile>>();

    bool producerCompleted = false;
    // Producer Task
    var producerTask = Task.Run(() =>
    {
        try
        {
            readAllFilesAtRootFolder(
                "H:/My Photos/IMages/2015 - Dhaham's Choir Concert",
                extensions, chkSubFolders.Checked, fileQueue,
                _cancellationTokenSource.Token);
        }
        finally
        {
            producerCompleted = true;
        }
    });

    var consumerTask = Task.Run(() => readAllEXIFdates(fileQueue, resultDictionary,
        _cancellationTokenSource.Token, producerCompleted));
    // Wait for both tasks to complete
    await Task.WhenAll(producerTask, consumerTask);

    label8.Text = $"Total files processed: {resultDictionary.Values.Count}";
}

The readAllEXIFdates() should exit when producerCompleted is changed to true but the readAllEXIFdates() loop never sees the update, even producerCompleted is updated as a public parameter to the class. Why is that ?

while (!cancellationToken.IsCancellationRequested
    && (!producerCompleted || !fileQueue.IsEmpty))

This is the while loop that does not exit since producerCompleted never get updated.

producerCompleted is a boolean value type - updating it inside a called method will not change the source variable. — Simon MᶜKenzie
– Simon MᶜKenzie, Commented Jan 13 at 22:57
Also, your sample code is not clear - readAllEXIFdates is never called. Can you update your question to contain the smallest possible code that reproduces your problem? — Simon MᶜKenzie
– Simon MᶜKenzie, Commented Jan 13 at 22:59
@PCG what you try to build is already provided by the built-in Channel<T> type. No need for thread-unsafe variables and checks, and processing can be done with await foreach(var file in channelReader.ReadAllFiles()){}. You could reduce all this code to about a dozen lines including proper exception handling, which is now missing — Panagiotis Kanavos
– Panagiotis Kanavos, Commented Jan 14 at 7:42
@TheodorZoulias the code looks like simple async processing of a bunch of files using the wrong types and a bit of hacky code. I suspect the OP found some bad old articles and tried to adopt them. — Panagiotis Kanavos
– Panagiotis Kanavos, Commented Jan 14 at 7:51

mjwills · Accepted Answer · 2025-01-14 06:04:19Z

2

You have done an enormous amount of work to basically simulate an existing framework type - the BlockingCollection. You should use this instead of a ConcurrentQueue.

Change your producer to Add to it, and then call CompleteAdding when done.

Change your consumer to foreach over GetConsumingEnumerable.

Done. That is all you need to do. GetConsumingEnumerable will automatically serve up the items added to it, and the foreach will exit once all queue items are processed. There is no need for producerCompleted etc.

edited Jan 14 at 6:04

answered Jan 14 at 5:52

mjwills

24.1k6 gold badges45 silver badges74 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Panagiotis Kanavos Jan 14 at 7:40

The appropriate asynchronous existing type is Channel, not BlockingCollection. Channel is a truly asynchronous pub/sub collection

mjwills Jan 14 at 8:02

@PanagiotisKanavos There certainly may be other alternatives - but the BlockingCollection is largely a drop in replacement for the existing code which needs a queue and a means to communicate "I'm done". BlockingCollection gives you that.

Michał Turczyn · Accepted Answer · 2025-01-14 08:10:34Z

You pass producerCompleted as parameter to a method, but bool is a value type, so when is passed as parameter, the method creates own copy of the vairable.

What you expect is behavior of reference type, where you pass variable around and can update it from other places.

For this kind scenario I would suggest defining "more global" variable, i.e.:

private volatile bool _producerCompleted = false;

so you use the same variable in both places - in a loop condition as well as in the method updating it.

Other thing that comes to my mind, you have:

(!producerCompleted || !fileQueue.IsEmpty)

Note that if fileQueue won't be empty, the loop would keep running regardless of producerCompleted.

EDIT

As correctly pointed out, the field could be used from different threads, that's why we need volatile, to prevent potential compiler optimizations.

volatile (C# Reference)

Collectives™ on Stack Overflow

Second Task does not end although loop parameter condition is updated in separate task

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related