Problem with C# Threads and Assert not Working

Question

I try to solve a problem in my program but I can't get done. The part which I have the problems is Modifier_le_solde_du_compte_a_partir_de_plusieurs_threads().

When I try to return the variable using Decimal Solde, it returns a random value instead of 0. The value returned in the assert must be equal to 0. Please help me.

This is the code in C#, Visual Studio.

Thanks you.

using System;

public class CompteBancaire {
    public decimal solde1;
    public bool bool_fermer = true;

    public void Ouvrir()
    {
        solde1 = 0;
    }

    public void Fermer()
    {
        if (solde1 <= 0)
        {
            bool_fermer = false;
        }
    }

    public decimal Solde
    {

        get
        {
            if (!bool_fermer)
            {
                throw new InvalidOperationException("");
            }
            return solde1;
        }
    }

    /// <summary>
    /// Mettre a jour le solde du compte bancaire.
    /// </summary>
    public void ReviserSolde(decimal change)
    {
        solde1 += change;
    }
}

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Xunit;

public class CompteBancaireTests
{
    [Fact]
    public void Retourne_un_solde_vide_apres_ouverture()
    {
        var compte = new CompteBancaire();
        compte.Ouvrir();

        Assert.Equal(0, compte.Solde);
    }

    [Fact]
    public void Verifier_le_solde()
    {
        var compte = new CompteBancaire();
        compte.Ouvrir();

        var solde_a_l_ouverture = compte.Solde;

        compte.ReviserSolde(10);
        var solde_mis_a_jour = compte.Solde;

        Assert.Equal(0, solde_a_l_ouverture);
        Assert.Equal(10, solde_mis_a_jour);
    }

    [Theory]
    [InlineData(10)]
    [InlineData(20)]
    public void Verifier_le_solde_random(int seed)
    {
        var compte = new CompteBancaire();
        compte.Ouvrir();

        var solde_a_l_ouverture = compte.Solde;

        var random = new Random(seed).Next(0, 100);

        compte.ReviserSolde(random);
        var solde_mis_a_jour = compte.Solde;

        Assert.Equal(0, solde_a_l_ouverture);
        Assert.Equal(random, solde_mis_a_jour);
    }

    [Fact]
    public void Le_solde_peut_augmenter_et_diminuer()
    {
        var compte = new CompteBancaire();
        compte.Ouvrir();
        var solde_a_l_ouverture = compte.Solde;

        compte.ReviserSolde(10);
        var ajouter_au_solde = compte.Solde;

        compte.ReviserSolde(-15);
        var soustraction_au_solde = compte.Solde;

        Assert.Equal(0, solde_a_l_ouverture);
        Assert.Equal(10, ajouter_au_solde);
        Assert.Equal(-5, soustraction_au_solde);
    }

    [Fact]
    public void Un_compte_ferme_lance_une_exception_lors_de_la_verification_du_solde()
    {
        var compte = new CompteBancaire();
        compte.Ouvrir();
        compte.Fermer();

        Assert.Throws<InvalidOperationException>(() => compte.Solde);
    }

    [Fact]
    public void Modifier_le_solde_du_compte_a_partir_de_plusieurs_threads()
    {
        var compte = new CompteBancaire();
        var liste_de_tasks = new List<Task>();

        const int nombre_de_threads = 500;
        const int iterations = 100;

        compte.Ouvrir();

        for (int i = 0; i < nombre_de_threads; i++)
        {
            liste_de_tasks.Add(Task.Run(() =>
            {
                for (int j = 0; j < iterations; j++)
                {
                    compte.ReviserSolde(1);
                    compte.ReviserSolde(-1);
                }
            }));
        }

        Task.WaitAll(liste_de_tasks.ToArray());

        Assert.Equal(0, compte.Solde);
    }
}

[synchronizing data for multithreading](https://learn.microsoft.com/en-us/dotnet/standard/threading/synchronizing-data-for-multithreading), for example with [lock statement](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement) — Guru Stron, Mar 31 '21 at 18:54
@GuruStron `solde1` is a `Decimal` but although not guaranteed to be thread-safe I don't understand where there could be a synchronisation problem given that the output is expected after `Task.WaitAll(...)`. — Martin, Mar 31 '21 at 19:02
@Martin multiple threads (see addition to `liste_de_tasks`) are modifying the same instance of `compte` via `ReviserSolde` (`+= ` is not an atomic operation) , so some synchronization is needed somewhere. I would say it should be done by "client", i.e. in the nested loop. — Guru Stron, Mar 31 '21 at 19:04
@GuruStron Yes I understood the code perfectly. However, the operation is performed using 500 tasks that each add and subtract 1 a hundred times. The end-result will be the same if you wait for all the tasks to finish, `0`. — Martin, Mar 31 '21 at 19:05
@Martin again, `+=` is not atomic, so for example two threads can simultaneously read current value (for example `0`) then add `1` and both place resulting `1` there, which gives `1` in `solde` after **2** additions. And then during subtractions no such overlap will happen resulting in `-1` at the end. — Guru Stron, Mar 31 '21 at 19:09

Guru Stron · Answer 1 · 2021-05-05T00:28:20.627

Operation += used in ReviserSolde is not atomic one and since it is invoked by multiple threads some synchronization is needed. For example you can modify your unit test so the caller will take care of it:

object locker = new object(); // create lock object
...

for (int i = 0; i < nombre_de_threads; i++)
{
    liste_de_tasks.Add(Task.Run(() =>
    {
        for (int j = 0; j < iterations; j++)
        {
            lock (locker) // lock the modification
            {
                compte.ReviserSolde(1);
                compte.ReviserSolde(-1);
            }
        }
    }));
}

Or you can make the ReviserSolde thread safe (for example adding lock there).

To dive a little bit deeper into this topic I usually recommend Albahari's Threading in C#.

FCR · Accepted Answer · 2021-04-01T09:09:00.540

0

EDIT: OP, my answer -as already pointed out by Guru Stron- is not what you are looking for. In order for it to work without locks it must become quite hacky and as it is right now, it defeats the purpose of multi-threading.

Your first for-loop is finishing before the spammed threads/tasks are done. This leads me to think that at some point your Task.WaitAll will find no task left (even if you intended to iterate further).

What works for me is moving Task.WaitAll(liste_de_tasks.ToArray()); into the first for-loop as follows:

for (int i = 0; i < nombre_de_threads; i++)
        {
            liste_de_tasks.Add(Task.Run(() =>
            {
                for (int j = 0; j < iterations; j++)
                {
                    compte.ReviserSolde(1);
                    compte.ReviserSolde(-1);
                    
                }
            }));
            Task.WaitAll(liste_de_tasks.ToArray());
        }

edited Apr 01 '21 at 09:09

answered Mar 31 '21 at 19:47

FCR

70
6

1

This will wait for every task sequentially basically removing all parallelism, so beating the purpose for all of the task creation and so on. – Guru Stron Mar 31 '21 at 20:42
you still get 'some' parallelism, just greatly tuned down. You can do this for example every 100 times for more if needed, though I get your point. – FCR Mar 31 '21 at 22:19
1

No, you will not. The first created task will be immediately waited, then the second and so on. – Guru Stron Mar 31 '21 at 22:37
As @GuruStron indicates, this answer is completely pointless. Do not use this solution. – mjwills Apr 01 '21 at 01:55

Problem with C# Threads and Assert not Working

2 Answers2