Parallel.ForEach changes behavior with more than one thread

Question

I have an array of files that are having write operations performed on them, I'm using a Parallel.ForEach loop to speed this up.

The problem is that when I have more than one thread, it behaves erratically. The program makes 10 writes of 250000 random bytes to WAV files, this will result in periods of about 1.3 seconds of static. What is happening is that with multiple threads, in the vast majority of files, there isn't any static, Windows indicates the files have changed, but the audio content has not, the other problem is that some clips had about 6 minutes of static written to it, which is impossible, the code only writes 2500000 bytes (~10.3 seconds) to each file, so for some reason its writing bytes to the wrong files, so some have no static, others have an absurd amount. Its impossible to miss the static, so it can't be me. I know something is going wrong.

Everything worked fine before I multithreaded the program, so I used MaxDegreeofParallelism = 1 and everything worked fine, so I know the problem is being caused by multiple threads.

Parallel.ForEach(files, new ParallelOptions{MaxDegreeOfParallelism = 4}, file =>
{
Random rand = new Random();
using (stream = new FileStream(file, FileMode.Open, FileAccess.ReadWrite))
{
FileInfo info = new FileInfo(file);
for (int i = 0; i < 10; i++)
{
    int pos = rand.Next(5000000, Convert.ToInt32(info.Length));
    for (int x = 0; x < 250000; x++)
    {
        byte number = array[rand.Next(array.Length)];
        stream.Seek(pos, SeekOrigin.Begin);
        pos += 4;
        stream.WriteByte(number);
    }
}
}
});

EDIT: The other problem is that with multiple threads it writes to the header portion, which contains the critical data, so no media player recognizes the format. This only happens with multiple threads, and I'm ignoring the first 5 million bytes, which I know is enough.

EDIT:2 Updated code.

Can anyone shed some light on what it is about multiple threads that causes my code to not function correctly?

Why writing a single byte for every four? Wouldn't that be 8bit noise in one channel? — spender, Aug 13 '12 at 02:02
I've added the rest of the Parallel.ForEach loop. I'm intentionally writing to just one channel at a time, that works properly. — 0_______0, Aug 13 '12 at 02:10
I think using() lead to unexpected behaviors. In .NET framework common language runtime (CLR) automatically release the memory when object is no longer used. There is always a thread running Garbage Collector to do that. There is crash between Parallel.ForEach() and using() statement — cat916, Aug 13 '12 at 02:25
I actually added the using block after the problem first occurred, it was happening without it initially, I was manually releasing the FileStream before. — 0_______0, Aug 13 '12 at 02:26

score 3 · Answer 1 · edited May 23 '17 at 11:56

3

Random number generator is not thread-safe. Jon Skeet has a good article on this and how to make a thread-safe random number generator.

edited May 23 '17 at 11:56

Community

1
1

answered Aug 13 '12 at 02:20

Jesse C. Slicer

19,901
3
68
87

I've updated the code, I hadn't copied everything. Rand is declared as thread static and a new instance is generated for each file; do I need to do more to keep it thread safe? – 0_______0 Aug 13 '12 at 02:24
1

I don't think that will work. Declare your random number generator in the `.ForEach()` with `Random rand = new Random();` rather than having it a class-level variable. I don't think `ThreadStaticAttribute` does anything if it's on a non-static variable (see: http://stackoverflow.com/questions/5227676/how-does-the-threadstatic-attribute-work#5227784) – Jesse C. Slicer Aug 13 '12 at 02:32
I tried instantiating rand in the `Parallel.ForEach` loop, but it had no effect, the problems still exist. But when I ran the test again it brought up another problem I'd forgotten about, writing the header part of the file, making it unreadable. The code in my question has been updated. – 0_______0 Aug 13 '12 at 03:14
@JesseC.Slicer I think that won't work well either. It's likely that several threads will have their copy of `Random` initialized with the same seed. Use Skeet's code instead. – svick Aug 13 '12 at 10:11

score 3 · Accepted Answer · answered Aug 13 '12 at 03:47

stream is declared outside of the delegate being executed by Parallel.ForEach. All the threads created by Parallel.ForEach are trying to use the same variable. Since there's no locking, it's undefined what value of stream each thread will have - you're probably seeing several threads writing to the same stream, which is resulting in your weird multi-threaded behavior.

Try removing the declaration of stream outside of the delegate, and changing

using (stream = new FileStream(file, FileMode.Open, FileAccess.ReadWrite))

to

using (Stream stream = new FileStream(file, FileMode.Open, FileAccess.ReadWrite))

I'm always staring right at the problem, but I just can't seem to see it. It is writing to all of the files properly now and the header is being kept intact, thanks. — 0_______0, Aug 13 '12 at 05:19

Parallel.ForEach changes behavior with more than one thread

2 Answers2