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);
    }
}

Answer 1

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#.

Answer 2

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());
        }