En proie à des bogues multithread

Dans ma nouvelle équipe que je gère, la majorité de notre code est la plate-forme, le socket TCP et le code de réseau http. Tout C ++. La plupart d'entre eux proviennent d'autres développeurs qui ont quitté l'équipe. Les développeurs actuels de l'équipe sont très intelligents, mais surtout juniors en termes d'expérience.

Notre plus gros problème: les bogues de concurrence multithread. La plupart de nos bibliothèques de classes sont écrites pour être asynchrones en utilisant certaines classes de pool de threads. Les méthodes sur les bibliothèques de classes mettent souvent en file d'attente des prises longues sur le pool de threads d'un thread, puis les méthodes de rappel de cette classe sont invoquées sur un autre thread. En conséquence, nous avons beaucoup de bogues de cas de bord impliquant des hypothèses de thread incorrectes. Il en résulte des bogues subtils qui vont au-delà de la simple présence de sections critiques et de verrous pour se prémunir contre les problèmes de concurrence.

Ce qui rend ces problèmes encore plus difficiles, c'est que les tentatives de résolution sont souvent incorrectes. Certaines erreurs que j'ai observées que l'équipe tente (ou dans le code hérité lui-même) incluent quelque chose comme ce qui suit:

Erreur courante # 1 - Résoudre le problème de concurrence en mettant simplement un verrou sur les données partagées, mais en oubliant ce qui se passe lorsque les méthodes ne sont pas appelées dans un ordre attendu. Voici un exemple très simple:

void Foo::OnHttpRequestComplete(statuscode status)
{
    m_pBar->DoSomethingImportant(status);
}

void Foo::Shutdown()
{
    m_pBar->Cleanup();
    delete m_pBar;
    m_pBar=nullptr;
}

Nous avons donc maintenant un bug dans lequel Shutdown pourrait être appelé pendant que OnHttpNetworkRequestComplete se produit. Un testeur trouve le bogue, capture le vidage sur incident et attribue le bogue à un développeur. Il corrige à son tour le bogue comme celui-ci.

void Foo::OnHttpRequestComplete(statuscode status)
{
    AutoLock lock(m_cs);
    m_pBar->DoSomethingImportant(status);
}

void Foo::Shutdown()
{
    AutoLock lock(m_cs);
    m_pBar->Cleanup();
    delete m_pBar;
    m_pBar=nullptr;
}

Le correctif ci-dessus semble bon jusqu'à ce que vous réalisiez qu'il existe un boîtier de bord encore plus subtil. Que se passe-t-il si Shutdown est appelé avant que OnHttpRequestComplete ne soit rappelé? Les exemples réels de mon équipe sont encore plus complexes et les cas marginaux sont encore plus difficiles à repérer pendant le processus de révision du code.

Erreur courante n ° 2 - résoudre les problèmes de blocage en sortant aveuglément du verrou, attendre la fin de l'autre thread, puis ressaisir le verrou - mais sans gérer le cas où l'objet vient d'être mis à jour par l'autre thread!

Erreur courante # 3 - Même si les objets sont comptés par référence, la séquence d'arrêt "libère" son pointeur. Mais oublie d'attendre que le thread en cours d'exécution libère son instance. En tant que tels, les composants sont arrêtés proprement, puis des rappels parasites ou tardifs sont invoqués sur un objet dans un état n'attendant plus d'appels.

Il existe d'autres cas de bord, mais la ligne de fond est la suivante:

La programmation multithread est tout simplement difficile, même pour les personnes intelligentes.

Pendant que j'attrape ces erreurs, je passe du temps à discuter des erreurs avec chaque développeur pour développer un correctif plus approprié. Mais je soupçonne qu'ils sont souvent confus sur la façon de résoudre chaque problème en raison de l'énorme quantité de code hérité que la «bonne» solution impliquera de toucher.

Nous allons être bientôt disponibles, et je suis sûr que les correctifs que nous appliquons seront valables pour la prochaine version. Ensuite, nous allons avoir du temps pour améliorer la base de code et refactoriser si nécessaire. Nous n'aurons pas le temps de tout réécrire. Et la majorité du code n'est pas si mal. Mais je cherche à refactoriser le code de manière à éviter complètement les problèmes de threading.

Une approche que j'envisage est la suivante. Pour chaque fonctionnalité de plate-forme importante, disposez d'un thread unique dédié sur lequel tous les événements et les rappels du réseau sont rassemblés. Similaire au filetage de cloisonnement COM dans Windows avec l'utilisation d'une boucle de message. Les opérations de blocage longues peuvent toujours être envoyées à un thread de pool de travail, mais le rappel de fin est invoqué sur le thread du composant. Les composants pourraient même partager le même thread. Ensuite, toutes les bibliothèques de classes exécutées à l'intérieur du thread peuvent être écrites sous l'hypothèse d'un monde à thread unique.

Avant de poursuivre dans cette voie, je suis également très intéressé par l'existence d'autres techniques ou modèles de conception standard pour traiter les problèmes multithreads. Et je dois souligner - quelque chose au-delà d'un livre qui décrit les bases des mutex et des sémaphores. Qu'est-ce que tu penses?

Je suis également intéressé par toute autre approche à adopter pour un processus de refactoring. Y compris l'un des éléments suivants:

1. Littérature ou articles sur les modèles de conception autour des fils. Quelque chose au-delà d'une introduction aux mutex et aux sémaphores. Nous n'avons pas non plus besoin d'un parallélisme massif, mais simplement de façons de concevoir un modèle objet afin de gérer correctement les événements asynchrones d'autres threads .

2. Façons de schématiser le filetage de divers composants, afin qu'il soit facile d'étudier et de faire évoluer des solutions. (C'est-à-dire un équivalent UML pour discuter des threads entre les objets et les classes)

3. Sensibiliser votre équipe de développement aux problèmes du code multithread.

4. Qu'est-ce que tu ferais?

Votre code a d' autres problèmes importants en dehors de cela. Supprimer manuellement un pointeur? Vous appelez une cleanupfonction? Owch. De plus, comme indiqué avec précision dans le commentaire de la question, vous n'utilisez pas RAII pour votre verrou, ce qui est un autre échec assez épique et garantit que lorsque DoSomethingImportantlève une exception, des choses terribles se produisent.

Le fait que ce bogue multithread se produise n'est qu'un symptôme du problème principal - votre code a une sémantique extrêmement mauvaise dans n'importe quelle situation de thread et vous utilisez des outils et des ex-idiomes complètement peu fiables. Si j'étais vous, je serais étonné qu'il fonctionne avec un seul thread, et encore moins.

Erreur courante # 3 - Même si les objets sont comptés par référence, la séquence d'arrêt "libère" son pointeur. Mais oublie d'attendre que le thread en cours d'exécution libère son instance. En tant que tels, les composants sont arrêtés proprement, puis des rappels parasites ou tardifs sont invoqués sur un objet dans un état qui n'attend plus d'autres appels.

Le point de référence entier est que le thread a déjà publié son instance . Parce que sinon, il ne peut pas être détruit car le thread a toujours une référence.

Utilisez std::shared_ptr. Lorsque toutes les discussions ont publié (et personne ne peuvent donc être appeler la fonction, car ils ont pas pointeur vers elle), puis le destructeur. Ceci est garanti en toute sécurité.

Deuxièmement, utilisez une véritable bibliothèque de threads, comme les Thread Building Blocks d'Intel ou la Parallel Patterns Library de Microsoft. Écrire le vôtre prend du temps et n'est pas fiable et votre code est plein de détails de thread dont il n'a pas besoin. Faire vos propres verrous est tout aussi mauvais que faire votre propre gestion de la mémoire. Ils ont déjà implémenté de nombreux idiomes de filetage très utiles à usage général qui fonctionnent correctement pour votre usage.

D'autres affiches ont bien commenté ce qui devrait être fait pour résoudre les problèmes fondamentaux. Ce message est préoccupé par le problème plus immédiat de corriger suffisamment le code hérité pour vous faire gagner du temps pour tout refaire de la bonne façon. En d'autres termes, ce n'est pas la bonne façon de faire les choses, c'est juste une façon de boiter pour le moment.

Votre idée de consolider les événements clés est un bon début. J'irais jusqu'à utiliser un seul thread de répartition pour gérer tous les événements de synchronisation clés, partout où il y a une dépendance à la commande. Configurez une file d'attente de messages thread-safe et partout où vous effectuez actuellement des opérations sensibles à la concurrence (allocations, nettoyages, rappels, etc.), envoyez plutôt un message à ce thread et faites-le exécuter ou déclencher l'opération. L'idée est que ce thread contrôle tous les démarrages, arrêts, allocations et nettoyages de l'unité de travail.

Le thread de répartition ne résout pas les problèmes que vous avez décrits, il les consolide simplement en un seul endroit. Vous devez toujours vous soucier des événements / messages se produisant dans un ordre inattendu. Les événements avec des durées d'exécution importantes devront toujours être envoyés à d'autres threads, il y a donc toujours des problèmes de concurrence sur les données partagées. Une façon d'atténuer cela est d'éviter de transmettre des données par référence. Dans la mesure du possible, les données contenues dans les messages d'expédition doivent être des copies qui appartiendront au destinataire. (Cela va dans le sens de rendre les données immuables que d'autres ont mentionnées.)

L'avantage de cette approche de répartition est qu'au sein du fil de répartition, vous disposez d'une sorte de valeur refuge où vous savez au moins que certaines opérations se produisent de manière séquentielle. L'inconvénient est qu'il crée un goulot d'étranglement et une surcharge de CPU supplémentaire. Je vous suggère de ne pas vous soucier de l'une de ces choses au début: concentrez-vous sur l'obtention d'une certaine mesure de fonctionnement correct en déplaçant autant que vous le pouvez dans le fil de répartition. Effectuez ensuite un profilage pour voir ce qui prend le plus de temps CPU et commencez à le déplacer hors du thread de répartition en utilisant les techniques de multithreading correctes.

Encore une fois, ce que je décris n'est pas la bonne façon de faire les choses, mais c'est un processus qui peut vous faire avancer dans la bonne direction par incréments suffisamment petits pour respecter les délais commerciaux.

Sur la base du code affiché, vous disposez d'un tas de WTF. Il est extrêmement difficile, voire impossible, de corriger progressivement une application multithread mal écrite. Dites aux propriétaires que l'application ne sera jamais fiable sans retouches importantes. Donnez-leur une estimation basée sur l'inspection et le remaniement de chaque bit du code qui interagit avec les objets partagés. Donnez-leur d'abord une estimation pour l'inspection. Ensuite, vous pouvez donner une estimation de la reprise.

Lorsque vous retravaillez le code, vous devez prévoir d'écrire le code afin qu'il soit correctement prouvé. Si vous ne savez pas comment faire, trouvez quelqu'un qui le fait, ou vous vous retrouverez au même endroit.

Si vous avez du temps à consacrer à la refactorisation de votre application, je vous conseille de jeter un œil au modèle d'acteur (voir par exemple Theron , Casablanca , libcppa , CAF pour les implémentations C ++).

Les acteurs sont des objets qui s'exécutent simultanément et communiquent entre eux uniquement à l'aide d'un échange de messages asynchrone. Ainsi, tous les problèmes de gestion des threads, mutex, blocages, etc., sont traités par une bibliothèque d'implémentation d'acteur et vous pouvez vous concentrer sur l'implémentation du comportement de vos objets (acteurs), ce qui revient à répéter la boucle

1. Recevoir un message
2. Effectuer le calcul
3. Envoyer des messages / créer / tuer d'autres acteurs.

Une approche pour vous pourrait être de faire d'abord de la lecture sur le sujet, et éventuellement de jeter un œil à une ou deux bibliothèques pour voir si le modèle d'acteur peut être intégré dans votre code.

J'utilise (une version simplifiée de) ce modèle dans un de mes projets depuis quelques mois maintenant et je suis étonné de sa robustesse.

Erreur courante # 1 - Résoudre le problème de concurrence en mettant simplement un verrou sur les données partagées, mais en oubliant ce qui se passe lorsque les méthodes ne sont pas appelées dans un ordre attendu. Voici un exemple très simple:

L'erreur ici n'est pas «l'oubli», mais le «non-correction». Si des choses se produisent dans un ordre inattendu, vous avez un problème. Vous devriez le résoudre au lieu d'essayer de le contourner (claquer un verrou sur quelque chose est généralement une solution de contournement).

Vous devez essayer d'adapter le modèle / la messagerie de l'acteur dans une certaine mesure et de séparer les préoccupations. Le rôle de Fooest clairement de gérer une sorte de communication HTTP. Si vous souhaitez concevoir votre système pour le faire en parallèle, c'est la couche supérieure qui doit gérer les cycles de vie des objets et accéder à la synchronisation en conséquence.

Il est difficile d'essayer de faire fonctionner un certain nombre de threads sur les mêmes données mutables. Mais c'est aussi rarement nécessaire. Tous les cas courants qui l'exigent ont déjà été résumés dans des concepts plus faciles à gérer et mis en œuvre un certain nombre de fois pour à peu près n'importe quel langage impératif majeur. Il vous suffit de les utiliser.

Vos problèmes sont assez mauvais, mais typiques d'une mauvaise utilisation de C ++. La révision du code résoudra certains de ces problèmes. 30 minutes, un jeu de globes oculaires donne 90% des résultats (la citation est googleable)

Problème n ° 1 Vous devez vous assurer qu'il existe une hiérarchie de verrouillage stricte pour éviter le blocage de votre verrouillage.

Si vous remplacez Autolock par un wrapper et une macro, vous pouvez le faire.

Conservez une carte globale statique des verrous créés à l'arrière de votre wrapper. Vous utilisez une macro pour insérer les informations de nom de fin et de numéro de ligne dans le constructeur d'encapsuleur Autolock.

Vous aurez également besoin d'un graphique de dominateur statique.

Maintenant, dans le verrou, vous devez mettre à jour le graphique de dominateur, et si vous obtenez un changement de commande, vous affirmez une erreur et abandonnez.

Après des tests approfondis, vous pouvez vous débarrasser de la plupart des blocages latents.

Le code est laissé comme exercice pour l'élève.

Le problème # 2 disparaîtra alors (principalement)

Votre solution archientctuelle va fonctionner. Je l'ai déjà utilisé dans des systèmes de mission et de vie. Mon point de vue est le suivant

• Passez des objets immuables ou faites-en des copies avant de passer.

• Ne partagez pas de données via des variables publiques ou des getters.

• Les événements externes arrivent via une répartition multithread dans une file d'attente desservie par un thread. Vous pouvez maintenant trier les raisons de la gestion des événements.

• Les modifications de données qui traversent les threads entrent dans une qeuue thread-safe, gérées par un thread. Faites des abonnements. Vous pouvez maintenant trier les raisons des flux de données.

• Si vos données doivent traverser la ville, publiez-les dans la file d'attente de données. Cela le copiera et le transmettra aux abonnés de manière asynchrone. Interrompt également toutes les dépendances de données dans le programme.

C'est à peu près un modèle d'acteur bon marché. Les liens de Giorgio vous aideront.

Enfin, votre problème avec les objets arrêtés.

Lorsque vous comptez les références, vous avez résolu 50%. Les 50% restants concernent le renvoi du nombre de rappels. Passez aux détenteurs de rappel une référence. L'appel d'arrêt doit alors attendre zéro compte sur le refcount. Ne résout pas les graphiques d'objets compliqués; c'est entrer dans la vraie collecte des ordures. (Quelle est la motivation de Java pour ne faire aucune promesse quant au moment ou si finalize () sera appelé; pour vous sortir de la programmation de cette façon.)

Pour les futurs explorateurs: pour compléter la réponse sur le modèle d'acteur, je voudrais ajouter CSP ( communicating sequential process ), avec un clin d'œil à la plus grande famille de calculs de processus dans laquelle il se trouve. CSP est similaire au modèle d'acteur, mais divisé différemment. Vous avez encore un tas de threads, mais ils communiquent via des canaux spécifiques, plutôt que spécifiquement entre eux, et les deux processus doivent être prêts à envoyer et à recevoir respectivement avant que l'un ou l'autre ne se produise. Il existe également un langage formalisé pour prouver que le code CSP est correct. Je suis toujours en transition vers l'utilisation intensive de CSP, mais je l'utilise dans quelques projets depuis quelques mois, maintenant, et les choses sont grandement simplifiées.

L'Université du Kent a une implémentation C ++ ( https://www.cs.kent.ac.uk/projects/ofa/c++csp/ , clonée sur https://github.com/themasterchef/cppcsp2 ).

Littérature ou articles sur les modèles de conception autour des fils. Quelque chose au-delà d'une introduction aux mutex et aux sémaphores. Nous n'avons pas non plus besoin d'un parallélisme massif, mais simplement de façons de concevoir un modèle objet afin de gérer correctement les événements asynchrones d'autres threads.

Je lis actuellement ceci et il explique tous les problèmes que vous pouvez obtenir et comment les éviter, en C ++ (en utilisant la nouvelle bibliothèque de threads mais je pense que les explications globales sont valables pour votre cas): http: //www.amazon. com / C-Concurrence-Action-Pratique-Multithreading / dp / 1933988770 / ref = sr_1_1? ie = UTF8 & qid = 1337934534 & sr = 8-1

Façons de schématiser le filetage de divers composants, afin qu'il soit facile d'étudier et de faire évoluer des solutions. (C'est-à-dire un équivalent UML pour discuter des threads entre les objets et les classes)

J'utilise personnellement un UML simplifié et suppose simplement que les messages sont effectués de manière asynchrone. En outre, cela est vrai entre les "modules", mais à l'intérieur des modules, je ne veux pas avoir à savoir.

Sensibiliser votre équipe de développement aux problèmes du code multithread.

Le livre aiderait, mais je pense que des exercices / prototypage et un mentor expérimenté seraient meilleurs.

Qu'est-ce que tu ferais?

J'éviterais totalement que des personnes ne comprenant pas les problèmes de concurrence travaillent sur le projet. Mais je suppose que vous ne pouvez pas le faire, donc dans votre cas spécifique, à part essayer de vous assurer que l'équipe est plus instruite, je n'en ai aucune idée.

Vous êtes déjà en route en reconnaissant le problème et en recherchant activement une solution. Voici ce que je ferais:

• Asseyez-vous et concevez un modèle de filetage pour votre application. Ceci est un document qui répond à des questions telles que: Quels types de fils avez-vous? Que faut-il faire dans quel fil? Quels différents types de modèles de synchronisation devez-vous utiliser? En d'autres termes, il devrait décrire les "règles d'engagement" lors de la lutte contre les problèmes de multithreading.
• Utilisez des outils d'analyse de threads pour vérifier votre base de code pour les erreurs. Valgrind a un vérificateur de threads appelé Helgrind qui est bon pour repérer des choses comme l'état partagé manipulé sans synchronisation appropriée. Il existe certainement d'autres bons outils, allez les chercher.
• Envisagez de migrer loin de C ++. C ++ est un cauchemar pour écrire des programmes simultanés. Mon choix personnel serait Erlang , mais c'est une question de goût.

En regardant votre exemple: Dès que Foo :: Shutdown commence à s'exécuter, il ne doit plus être possible d'appeler OnHttpRequestComplete pour s'exécuter. Cela n'a rien à voir avec une implémentation, cela ne peut tout simplement pas fonctionner.

Vous pouvez également affirmer que Foo :: Shutdown ne devrait pas être appelable pendant qu'un appel à OnHttpRequestComplete est en cours d'exécution (certainement vrai) et probablement pas si un appel à OnHttpRequestComplete est toujours en attente.

La première chose à faire n'est pas de verrouiller etc., mais la logique de ce qui est autorisé ou non. Un modèle simple serait que votre classe peut avoir zéro ou plusieurs demandes incomplètes, zéro ou plusieurs achèvements qui n'ont pas encore été appelés, zéro ou plusieurs achèvements en cours d'exécution, et que votre objet souhaite s'arrêter ou non.

Foo :: Shutdown devrait terminer l'exécution des finitions, exécuter des requêtes incomplètes au point où elles peuvent être arrêtées si possible, ne plus autoriser le démarrage de plus de finitions, ne plus autoriser le démarrage de plus de requêtes.

Ce que vous devez faire: Ajoutez des spécifications à vos fonctions en indiquant exactement ce qu'elles feront. (Par exemple, le démarrage d'une requête http peut échouer après l'appel de hase Shutdown). Ensuite, écrivez vos fonctions pour qu'elles correspondent aux spécifications.

Les verrous ne doivent être utilisés que le moins longtemps possible pour contrôler la modification des variables partagées. Vous pourriez donc avoir une variable "performantShutDown" qui est protégée par un verrou.

Qu'est-ce que tu ferais?

Pour être honnête; Je m'enfuirais rapidement.

Les problèmes de concurrence sont NASTY . Quelque chose peut fonctionner parfaitement pendant des mois, puis (en raison du calendrier spécifique de plusieurs choses) exploser soudainement au visage du client, sans aucun moyen de comprendre ce qui s'est passé, aucun espoir de voir un beau rapport de bogue (reproductible) et aucun moyen pour être sûr que ce n'était pas un problème matériel qui n'a rien à voir avec le logiciel.

Éviter les problèmes de concurrence doit commencer pendant la phase de conception, en commençant par exactement comment vous allez le faire ("ordre de verrouillage global", modèle d'acteur, ...). Ce n'est pas quelque chose que vous essayez de résoudre dans une panique folle dans l'espoir que tout ne s'autodétruit pas après une prochaine sortie.

Notez que je ne plaisante pas ici. Vos propres mots («la plupart proviennent d'autres développeurs qui ont quitté l'équipe. Les développeurs actuels de l'équipe sont très intelligents, mais surtout juniors en termes d'expérience. ») Indiquent que toutes les personnes d'expérience ont déjà fait ce que je suis en train de suggérer.