Member Avatar for Tellalca

Hey guys, I am playing with the .NET 4.0's new class Parallel.

I tried to open files in a directory and calculate the total bytes of them. However when I run the code, I get a different result every time.

Can you explain me the problem I have?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using System.Diagnostics;
using System.Runtime.Remoting.Messaging;
using System.IO;
using System.Drawing;

namespace LearnCSharp
{

    class Program
    {
        static long totalBytes = 0;

        private static object token = new object();

        static void ProcessImages()
        {
            string directoryPath = @"C:\files";

            string[] fileNames = Directory.GetFiles(directoryPath);

            if (fileNames == null)
                throw new Exception("Path is wrong");

            ParallelLoopResult result = Parallel.ForEach<string>(fileNames, new Action<string>(
                delegate(string fileName)
                {   
                    FileStream stream = File.OpenRead(fileName);
                    if (stream == null)
                        throw new Exception("File not found");

                    lock (token)
                    {
                        totalBytes =+ stream.Length;  
                    }
                    
                }
            ));

        }

        static void Main(string[] args)
        {
            TaskFactory taskFactory = new TaskFactory();
            Task task =  taskFactory.StartNew(new Action(ProcessImages));

            Task.WaitAll(task);

            Console.WriteLine(totalBytes);

            Console.Read();
        }
    }
}
  • 3 Contributors
  • 5 Replies
  • 224 Views
  • 13 Hours Discussion Span
  • Latest Post Latest Post by Momerath

Dani AI

Generated

The root cause identified by and clarified by is correct, but a few important practical points and safer patterns are worth adding.

The expression with =+ compiles because the + on the right is the unary plus operator, so the statement assigns the file length (not add it) and the last parallel task to write wins. The lock in the original loop only serialized those assignments, so it did not make the arithmetic correct. Two immediate fixes are: (1) actually perform an addition (+=) or (2) perform an atomic add or thread-local aggregation so parallel workers don't contend.

Avoid opening a FileStream just to get Length when a FileInfo(file).Length will do and avoids needing to dispose streams. If a stream must be opened, use a using to ensure it gets closed, and compute the length outside any lock so the critical section is tiny.

Safe examples:

using System.Threading;

// atomic add (no explicit lock)
long len = new FileInfo(fileName).Length;
Interlocked.Add(ref totalBytes, len);
 // per-thread local aggregation (reduces contention)
Parallel.ForEach(fileNames,
    () => 0L,
    (file, state, localSum) => { localSum += new FileInfo(file).Length; return localSum; },
    localSum => Interlocked.Add(ref totalBytes, localSum)
);

If Interlocked.Add is not an option, keep the lock scope minimal:

long len = new FileInfo(file).Length;
lock(token) totalBytes += len;

Other notes: avoid long-running work (file I/O) inside locks, handle IO exceptions (files can change or be locked by other processes), and confirm types (use long for byte totals). These changes fix the nondeterministic results and make the parallel aggregation both correct and more scalable.

Recommended Answers

All 5 Replies

Member Avatar for skatamatic

Is this code copy + pasted? I have never seen a =+ operator before (as on line 39). How are the results different every time?

Member Avatar for Tellalca

Okay, you can assume it as +=.

The results I get are like 150907, 220769, 78905 and so on. They are different each time I run the application. I do not add or remove any files from the directory.

What can be the problem other than =+ :D

Member Avatar for Tellalca

, Wow the stupid mistake you found in the code was the actual problem.

Thank you :)

Member Avatar for skatamatic

Lol glad I could help. I am curious why this even compiled though....

Edit: I typed that into visual studio and it kind of makes sense
Basically the + is being thought of as a positive integer (rather than - indicating a negative).
So totalBytes =+ stream.Length means totalBytes = 0 + stream.Length. Therefore TotalBytes equals whichever parallel task's stream size that finishes last.

Edited by skatamatic because: n/a
Member Avatar for Momerath

Just to clarify that, the + is the unary +, just like there is the unary - (for example, x = x + -1, x = x + +1). It's rarely used, as in I've never seen anyone use it :)

commented: Beats my explanation lol. +9
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.