5
votes

Visibilité de mise à jour des membres C ++ dans une section critique lorsqu'elle n'est pas atomique

Je suis tombé sur le StackExchange de révision de code suivant et j'ai décidé de le lire pour m'entraîner . Dans le code, il y a ce qui suit:

Remarque: Je ne recherche pas de révision de code et il s'agit simplement d'un copier-coller du code à partir du lien afin que vous puissiez vous concentrer sur sur le problème en question sans que l'autre code n'interfère. Je ne suis pas intéressé par l'implémentation d'un 'pointeur intelligent', juste comprendre le modèle de mémoire:

#include <atomic>
#include <mutex>

struct ClassNameHere {
    int* rawObj;
    std::atomic<unsigned int> count;
    std::mutex mutex;

    // ...

    void deref()
    {
        std::scoped_lock lock{mutex};
        count--;
        if (count == 0)
            delete rawObj;
    }
};

Voir cela me fait immédiatement penser "et si deux threads entraient quand count == 1 et ni l'un ni l'autre ne voient les mises à jour l'un de l'autre? Les deux peuvent-ils finir par voir count comme zéro et une double suppression? Et est-il possible que deux threads provoquent count pour devenir -1 et ensuite la suppression ne se produit jamais?

Le mutex s'assurera qu'un thread entre dans la section critique, mais cela garantit-il que tous les threads seront correctement mis à jour? Que me dit le modèle de mémoire C ++ pour que je puisse dire que c'est une condition de concurrence ou non?

J'ai regardé le Page de référence du modèle de mémoire et std :: memory_order cppreference , mais cette dernière page semble traiter d'un paramètre pour atomic. Je n'ai pas trouvé la réponse que je cherchais ou je l'ai peut-être mal interprétée. Quelqu'un peut-il me dire si ce que j'ai dit est faux ou juste, et si ce code est sûr ou non?

Pour corriger le code s'il est cassé: p >

La bonne réponse pour cela est-elle de transformer le compte en un membre atomique? Ou est-ce que cela fonctionne et après avoir libéré le verrou sur le mutex, tous les threads voient la valeur?

Je suis également curieux de savoir si cela serait considéré comme la bonne réponse:

Remarque: je ne recherche pas de révision de code et j'essaie de voir si ce type de solution résoudrait le problème concernant le modèle de mémoire C ++.

// Copied from the link provided (all inside a class)

unsigned int count;
mutex m_Mutx;

void deref()
{
    m_Mutx.lock();
    count--;
    m_Mutx.unlock();
    if (count == 0)
    {
        delete rawObj;
        count = 0;
    }
}


2 commentaires

Vous demandez partiellement une révision du code, vous connaissez un meilleur endroit pour cela. De plus, votre type ClassNameHere contient un groupe de membres publics. Aussi, ne mettez pas de balises dans le titre, c'est à cela que servent les balises. Enfin, vous posez ici plusieurs questions (liées). La règle générale est de n'en poser qu'une.


@UlrichEckhardt Je ne veux pas du tout de révision de code. Il s'agit d'un code temporaire conçu pour compiler et illustrer rapidement un problème. Je ne suis pas sûr que le fait d'être à l'intérieur d'une structure / classe signifie quelque chose de différent d'une variable dans l'espace de noms global, donc je l'ai juste enveloppé dans une structure. Tout ce que je veux savoir , c'est si le modèle de mémoire C ++ confirme ou nie qu'il y a des problèmes ici. Le code que j'ai écrit est un point de départ et pour aider à illustrer le problème en question. J'encourage toute personne lisant ceci à ne pas faire une révision de code mais à se concentrer sur le modèle de mémoire. Je modifierai mon message pour le rendre plus clair.


4 Réponses :


2
votes

Dans les deux cas, il y a une course aux données. Le thread 1 décrémente le compteur à 1, et juste avant l'instruction if , un changement de thread se produit. Thread 2 décrémente le compteur à 0, puis supprime l'objet. Le thread 1 reprend, voit que count est égal à 0 et supprime à nouveau l'objet.

Déplacez le unlock () à la fin de la fonction ou, mieux, utilisez std :: lock_guard pour faire le verrouillage; son destructeur déverrouillera le mutex même lorsque l'appel de suppression lèvera une exception.


0 commentaires

4
votes

"et si deux threads entrent quand count == 1" - si cela se produit, quelque chose d'autre est louche. L'idée derrière les pointeurs intelligents est que le refcount est lié à la durée de vie (portée) d'un objet. Le décrément se produit lorsque l'objet (via le déroulement de la pile) est détruit. Si deux threads déclenchent cela, le refcount ne peut pas être juste 1 à moins qu'un autre bogue ne soit présent.

Cependant, ce qui pourrait arriver, c'est que deux threads entrent ce code lorsque count = 2 . Dans ce cas, l'opération de décrémentation est verrouillée par le mutex, elle ne peut donc jamais atteindre des valeurs négatives. Encore une fois, cela suppose du code non bogué ailleurs. Comme tout cela ne fait que supprimer l'objet (puis définir de manière redondante count à zéro), rien de mal ne peut arriver.

Ce qui peut arriver, c'est une double suppression. Si deux threads à count = 2 décrémentent le nombre, ils pourraient tous les deux voir le count = 0 par la suite. Déterminez simplement s'il faut supprimer l'objet à l'intérieur du mutex comme solution simple. Stockez ces informations dans une variable locale et gérez-les en conséquence après la libération du mutex.

Concernant votre troisième question, transformer le décompte en atomique ne va pas régler les choses par magie. De plus, le point derrière atomics est que vous n'avez pas besoin d'un mutex, car le verrouillage d'un mutex est une opération coûteuse. Avec atomics, vous pouvez combiner des opérations telles que décrémenter et vérifier zéro, ce qui est similaire au correctif proposé ci-dessus. Les données atomiques sont généralement plus lentes que les entiers «normaux». Cependant, ils sont toujours plus rapides qu'un mutex.


2 commentaires

Je ne suis pas sûr de ce que vous entendez par Je vérifie simplement si count == 1 à l'intérieur du mutex est une solution simple . Le chèque doit toujours être contre 0 . Vous voulez peut-être dire le faire avant la décrémentation? (mais ce serait une façon étrange de l'écrire).


De plus, si deux threads font entrer avec count == 1 suite à un autre échec, alors rawObj peut pas être supprimé. Je m'attendrais même à cette possibilité, avec le code pour initialiser / incrémenter le compteur reflétant la même compréhension erronée du verrouillage.



1
votes

Votre verrou empêche que l'opération count-- ne se mette en désordre lorsqu'elle est exécutée simultanément dans différents threads. Cela ne garantit pas, cependant, que les valeurs de count sont synchronisées, de sorte que des lectures répétées en dehors d'une seule section critique porteront le risque d'une course aux données.

Vous pouvez le réécrire comme suit:

void deref()
{
    std::lock_guard<std::mutex> lock(m_Mutx);
    if (! --count) {
        delete rawObj;
    }
}

De ce fait, le verrou garantit que l'accès à count est synchronisé et toujours dans un état valide . Cet état valide est reporté à la section non critique via une variable locale (sans condition de concurrence). Ainsi, la section critique peut être assez courte.

Une version plus simple serait de synchroniser le corps complet de la fonction; cela pourrait avoir un inconvénient si vous voulez faire des choses plus élaborées que simplement supprimer rawObj :

void deref()
{
    bool isLast;
    m_Mutx.lock();
    --count;
    isLast = (count == 0);
    m_Mutx.unlock();
    if (isLast) {
        delete rawObj;
    }
}

BTW: std :: atomic allone ne résoudra pas ce problème car cela ne synchronise que chaque accès unique, mais pas une "transaction". Par conséquent, votre scoped_lock est nécessaire, et - comme cela couvre la fonction complète alors - le std :: atomic devient superflu.


5 commentaires

Il n'y a pas besoin d'un entier qui reste non initialisé. Utilisez simplement --count; sur une ligne et bool last = (count == 0); dans la suivante. Après avoir déverrouillé le mutex, vérifiez cet indicateur. Si vous le souhaitez, vous pouvez également stocker une copie du compteur après la décrémentation. Cependant, le conditionnel ne doit pas être nécessaire: à condition que le code restant soit sain d'esprit, deref () ne doit pas être appelé lorsque count = 0 , de peur d'obtenir une double suppression.


@Ulrich Eckhardt; droite; voulait garder le code plus proche de cet OP, mais je suis d'accord: transporter uniquement les informations essentielles, c'est-à-dire le drapeau isLast , c'est mieux.


Il n'est toujours pas nécessaire de déclarer la variable sans l'initialiser. ;)


@Uldrich Eckhardt Quel conditionnel est inutile? Vous devez créer une branche quelle que soit la façon dont vous le faites.


@UlrichEckhardt Ah, vous parliez du ternaire qu'il avait auparavant dans la section critique. Merci, bravo.



1
votes

Si deux threads potentiellement * entrent deref () simultanément, alors, quelle que soit la valeur précédente ou précédemment attendue de count , une course aux données se produit, et votre programme entier , même les parties que vous vous attendriez à être chronologiquement antérieures, a un comportement indéfini comme indiqué dans le Norme C ++ dans [intro.multithread / 20] (N4659):

Deux actions sont potentiellement simultanées si

(20.1) ils sont exécutés par différents threads, ou

(20.2) ils ne sont pas séquencés, au moins un est exécuté par un gestionnaire de signaux, et ils ne sont pas tous les deux exécutés par le même appel de gestionnaire de signaux.

L'exécution d'un programme contient une course aux données s'il contient deux actions potentiellement conflictuelles, dont au moins pas atomique, et ni l'un ni l'autre ne se produit avant l'autre, à l'exception du cas particulier des gestionnaires de signaux décrit ci-dessous. Une telle course aux données entraîne un comportement indéfini.

Les actions potentiellement simultanées dans ce cas, bien sûr, sont la lecture de count en dehors de la section verrouillée, et l'écriture de count à l'intérieur.

*) Autrement dit, si les entrées actuelles le permettent.

MISE À JOUR 1 : la section à laquelle vous faites référence, décrivant l'ordre de la mémoire atomique, explique comment les opérations atomiques se synchronisent entre elles et avec d'autres primitives de synchronisation (telles que les mutex et les barrières de mémoire). En d'autres termes, il décrit comment les atomiques peuvent être utilisés pour la synchronisation afin que certaines opérations ne soient pas des courses de données. Cela ne s'applique pas ici. La norme adopte ici une approche conservatrice: à moins que d'autres parties de la norme ne précisent explicitement que deux accès conflictuels ne sont pas simultanés, vous avez une course aux données, et donc UB (où conflit signifie le même emplacement mémoire, et au moins l'un d'entre eux n'est pas t lecture seule).


0 commentaires