mik4n
mik4n

Reputation: 63

C# MultiThread without duplication

This is my code:

    Thread _th1, _th2, _th3, _th4;
int _i;

    void thMethod()
    {
        while(_i < 100){
            Thread.Sleep((new Random()).Next(1,500));
            Application.DoEvents();
            Console.WriteLine(_i);
            _i++;
        }
    }

    private void button4_Click(object sender, EventArgs e)
    {
        _th1 = new Thread(new ThreadStart(thMethod));
        _th1.Start();
        _th2 = new Thread(new ThreadStart(thMethod));
        _th2.Start();
        _th3 = new Thread(new ThreadStart(thMethod));
        _th3.Start();
        _th4 = new Thread(new ThreadStart(thMethod));
        _th4.Start();
    }

What i want to do is, doing Console.WriteLine from 0 to 99 using multithread, and the delay is random.

However, my code prints duplicated numbers

well, this is the result.

0 1 2 2 4 5 6 7 8 9 10 10 12 13 14 14 16 16 18 19 20 20 22 22 24 24 26 26 28 28 30 31 32 33 34 35 36 36 38 39 40 40 42 42 44 44 46 46 48 49 50 50 52 52 54 54 56 56 58 58 60 60 62 62 64 65 66 66 68 68 70 70 72 72 74 74 76 76 78 78 80 80 82 82 84 84 86 87 88 88 90 90 92 92 94 94 96 96 98 98 100 101 102

As you see, there are duplicated numbers printed, and it does not even stop at 99.

So... How to correct this code in order to make it run properly?

====================== Do i have to change like..

    void thMethod()
    {
        while(_i < 100){
            Thread.Sleep((new Random()).Next(1,500));
            Application.DoEvents();
            Interlocked.Increment(ref _i);
            Console.WriteLine(_i);
        }
    }

this?

Upvotes: 2

Views: 1553

Answers (6)

Platypus
Platypus

Reputation: 61

I solved the problem with interlocked. This is a vb.net console app, but is easily understandable. Every thread gets its unique counter, obviously the output sequence is not ordered


Module Module1

Dim count As Integer = 0

Sub Main()
    For i = 1 To 10
        Dim th As New Thread(AddressOf ThreadedIncrement)
        th.Start()
    Next
    Console.ReadLine()
End Sub

Sub ThreadedIncrement()
    Dim r As New Random

    For i = 1 To 10
        Thread.SpinWait(r.Next(1000, 1000000))
        Dim ncount As Integer = Interlocked.Add(count, 1)
        Console.WriteLine(ncount.ToString)
    Next
End Sub

End module


Upvotes: -1

Conrad Frix
Conrad Frix

Reputation: 52645

There are two options other then using Monitor that weren't explored

If you're just incrementing an integral type using one of the Interlocked methods offers the best performance. Since you're also using this variable to signal completion Interlocked.CompareExchange is a natural fit. This technique is used in lock-free and wait-free solutions.

while (_i < 100)
{
    Thread.Sleep((new Random()).Next(1, 500));
    int initI;
    int modifiedI;
    bool valueUpdated = false;
    do
    {
        initI = _i;
        modifiedI = initI + 1;

        if (modifiedI > 100)
            break;

        valueUpdated =  (initI == Interlocked.CompareExchange(
                ref _i, modifiedI, initI) );

        if (valueUpdated)
            Console.WriteLine(modifiedI);

    } while (!valueUpdated)  ;


}

If you're going for simplicity you can also use the Parallel Class

Parallel.For(0, 100, i =>
    {
        Thread.Sleep((new Random()).Next(1, 500));
        _i++;
        Console.WriteLine("{0} | {1}" ,Thread.CurrentThread.ManagedThreadId, _i );
    }
);

For demonstration purposes I added the output of the ManagedThreadId so you can see that more than one thread was in use.

You should note that you lose direct control of the number of threads created instead you can pass in ParallelOption with the MaxDegreeOfParallelism set which does influence the number of threads .

Upvotes: 0

David Esteves
David Esteves

Reputation: 1604

Although you should be using Interlocked.Increment as mentioned by other posts. This isn't the exact issue you're having.

The reason your numbers go past 100 is because the line you are incrementing the value comes after the Sleep. This causes threads to enter the loop, but by the time they write out the value the value has already changed to be above 100.

For example t1, t2, and t3 enter the loop and wait for 500ms and i is 99. Then t4 exits and increments it to 100, now t1 will print 100, t2 will print 101, t3 will print 102.

Interlocked.Increment wont actually solve the issue because the Thread.Sleep will still cause inconsistencies when the value is actually written out.

To solve the problem you would need to use a thread lock declare a variable to lock on as you can't lock on _i as its not a reference type:

object obj = new object();

void thMethod()
{
    while (_i < 100)
    {
        lock (obj)
        {
            Console.WriteLine(_i);
            _i++;
        }
        Thread.Sleep((new Random()).Next(1, 500));
    }
}

This way you will get no duplicates and the value will stop at 99.

Upvotes: 3

Amirshk
Amirshk

Reputation: 8258

In a multi-thread program, a shared resource with multiple writers needs some kind of a synchronization mechanism.

What happens is several threads are accessing the variable for printing before one of them has the chance to increment it, aka race condition.

To solve this you need to increment the variable in a critical section:

    void thMethod()
    {
        while (_i < 100)
        {
            Thread.Sleep((new Random()).Next(1, 500));
            Application.DoEvents();
            lock(this)
            {
                Console.WriteLine(_i);
                _i++;
            }
        }
    }

Now this won't stop at 100, so another fix will be:

        while (_i < 100)
        {
            Thread.Sleep((new Random()).Next(1, 500));
            Application.DoEvents();
            lock(this)
            {
                if (_i >= 100) break;
                Console.WriteLine(_i);
                _i++;
            }
        }

Upvotes: 1

bittenbytailfly
bittenbytailfly

Reputation: 233

The variable _i must be declared in the thMethod, not at a global level.

Upvotes: 0

SLaks
SLaks

Reputation: 887413

You need to call Interlocked.Increment to increment i in a thread-safe fashion.

Upvotes: 1

Related Questions