From 97d579e142ec075462833d348ea22fe6d3a1b628 Mon Sep 17 00:00:00 2001 From: StillHammer Date: Fri, 21 Nov 2025 19:50:42 +0800 Subject: [PATCH] docs: Add implementation prompt for deadlock prevention plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create comprehensive prompt for successor developer/agent to implement the 15h deadlock detection & prevention plan. Includes: - Complete implementation roadmap (Semaine 1: Detection, Semaine 2: Prevention) - Step-by-step commands for each phase - Code examples BEFORE/AFTER - Success criteria and validation checklist - Debugging tips and common pitfalls - Final report template This prompt is fully autonomous - a fresh developer can execute the entire plan following this guide. đŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- docs/plans/PROMPT_implement_deadlock_plan.md | 609 +++++++++++++++++++ 1 file changed, 609 insertions(+) create mode 100644 docs/plans/PROMPT_implement_deadlock_plan.md diff --git a/docs/plans/PROMPT_implement_deadlock_plan.md b/docs/plans/PROMPT_implement_deadlock_plan.md new file mode 100644 index 0000000..bc3e77f --- /dev/null +++ b/docs/plans/PROMPT_implement_deadlock_plan.md @@ -0,0 +1,609 @@ +# Prompt : ImplĂ©mentation du Plan Deadlock Detection & Prevention + +**Pour** : Claude Code (successeur) ou dĂ©veloppeur qui reprend ce plan +**Plan associĂ©** : [PLAN_deadlock_detection_prevention.md](./PLAN_deadlock_detection_prevention.md) +**DurĂ©e estimĂ©e** : 15h sur 2 semaines +**DerniĂšre mise Ă  jour** : 2025-01-21 + +--- + +## 🎯 Mission + +Tu dois implĂ©menter un systĂšme complet de dĂ©tection et prĂ©vention des deadlocks pour GroveEngine, un moteur de jeu avec hot-reload de modules dynamiques en C++17. + +**Contexte** : +- GroveEngine utilise actuellement `std::mutex` avec `std::lock_guard` +- Pattern : Single mutex par classe (risque faible mais amĂ©lioration nĂ©cessaire) +- Un deadlock a Ă©tĂ© identifiĂ© dans `test_13_cross_system` (teste encore bloquĂ©) +- Le projet compile sur Linux/WSL2 avec CMake + +**Objectif final** : +1. ✅ DĂ©tection automatique des deadlocks (TSan + Helgrind) +2. ✅ PrĂ©vention des deadlocks multi-mutex (scoped_lock) +3. ✅ Optimisation concurrence read-heavy (shared_mutex) +4. ✅ Tous les tests passent sans deadlock + +--- + +## 📚 Documentation Requise + +**IMPORTANT** : Lis ces documents avant de commencer : + +1. **Plan complet** : `docs/plans/PLAN_deadlock_detection_prevention.md` + - Contient TOUTES les modifications ligne par ligne + - Exemples de code AVANT/APRÈS + - Commandes exactes Ă  exĂ©cuter + +2. **Tests d'intĂ©gration** : `docs/plans/PLAN_integration_tests_global.md` + - Liste des 13 tests Ă  valider + - Seuils de succĂšs + +3. **Architecture** : `docs/architecture/` + - Comprendre IntraIO, IntraIOManager, TopicTree + - Flux de messages pub/sub + +--- + +## đŸ—“ïž Calendrier d'ImplĂ©mentation + +### Semaine 1 : DĂ©tection (5h) + +#### Jour 1-2 : ThreadSanitizer (2h) + +**Checklist** : +- [ ] Modifier `CMakeLists.txt` (copier le code du plan lignes 21-35) +- [ ] Build avec TSan : `cmake -DGROVE_ENABLE_TSAN=ON -B build-tsan` +- [ ] ExĂ©cuter tous les tests : `cd build-tsan && TSAN_OPTIONS="detect_deadlocks=1" ctest -V` +- [ ] Documenter les warnings TSan trouvĂ©s (crĂ©er `docs/tsan_findings.md`) +- [ ] Fixer tous les lock-order-inversions dĂ©tectĂ©s +- [ ] Re-run tests jusqu'Ă  0 warning + +**Commandes** : +```bash +# 1. Modifier CMakeLists.txt (copier du plan) +nano CMakeLists.txt + +# 2. Build +cmake -DGROVE_ENABLE_TSAN=ON -B build-tsan +cmake --build build-tsan -j$(nproc) + +# 3. Test +cd build-tsan +TSAN_OPTIONS="detect_deadlocks=1 history_size=7 exitcode=1" ctest --output-on-failure + +# 4. Si erreurs, voir logs +cat Testing/Temporary/LastTest.log +``` + +**CritĂšres de succĂšs** : +- ✅ Build TSan compile sans erreurs +- ✅ `ctest` retourne exit code 0 +- ✅ Aucun warning "lock-order-inversion" +- ✅ Aucun warning "data race" + +--- + +#### Jour 3-4 : Helgrind (3h) + +**Checklist** : +- [ ] Modifier `CMakeLists.txt` (copier code du plan lignes 84-126) +- [ ] CrĂ©er `helgrind.supp` (copier du plan lignes 128-158) +- [ ] Build : `cmake -DGROVE_ENABLE_HELGRIND=ON -B build` +- [ ] ExĂ©cuter : `cd build && make helgrind` +- [ ] Analyser `helgrind-full.log` +- [ ] Ajouter suppressions pour false positives +- [ ] Re-run jusqu'Ă  log propre +- [ ] CrĂ©er tableau comparatif TSan vs Helgrind + +**Commandes** : +```bash +# 1. CrĂ©er helgrind.supp (copier du plan) +nano helgrind.supp + +# 2. Modifier CMakeLists.txt +nano CMakeLists.txt + +# 3. Build +cmake -DGROVE_ENABLE_HELGRIND=ON -B build +cmake --build build + +# 4. Run Helgrind (LENT - 10-50x slowdown) +cd build +make helgrind + +# 5. Analyser rĂ©sultats +cat helgrind-full.log | grep -E "(Possible|lock order)" | less + +# 6. Compter problĂšmes +cat helgrind-full.log | grep "Possible data race" | wc -l +``` + +**CritĂšres de succĂšs** : +- ✅ Target `make helgrind` fonctionne +- ✅ Fichier helgrind.supp avec suppressions +- ✅ 0 lock order violations (aprĂšs suppressions lĂ©gitimes) +- ✅ Tableau comparatif TSan vs Helgrind documentĂ© + +--- + +### Semaine 2 : PrĂ©vention & Optimisation (10h) + +#### Jour 5-7 : std::scoped_lock (4h) + +**Checklist** : +- [ ] Rechercher tous les lock_guard multi-mutex : `grep -n "lock_guard" src/*.cpp | sort` +- [ ] Identifier les patterns : lock(mutex1) puis lock(mutex2) dans mĂȘme fonction +- [ ] Refactorer `IntraIOManager.cpp` lignes 176, 221, 256, 272, 329 +- [ ] Remplacer par `std::scoped_lock(mutex1, mutex2)` +- [ ] CrĂ©er test unitaire `tests/unit/test_scoped_lock.cpp` (copier du plan) +- [ ] Build et tester : `cmake --build build && ctest` +- [ ] Valider avec TSan : aucun lock-order-inversion + +**Exemple de refactoring** : + +**AVANT** (src/IntraIOManager.cpp:176) : +```cpp +void IntraIOManager::routeMessage(...) { + std::lock_guard lock(managerMutex); + // ... code ... + std::lock_guard batchLock(batchMutex); // ❌ Risque deadlock +} +``` + +**APRÈS** : +```cpp +void IntraIOManager::routeMessage(...) { + std::scoped_lock lock(managerMutex, batchMutex); // ✅ Safe + // ... code ... +} +``` + +**Fichiers Ă  modifier** : +1. `src/IntraIOManager.cpp` (5 endroits - voir plan lignes 213-234) +2. `tests/unit/test_scoped_lock.cpp` (crĂ©er nouveau fichier - code dans plan lignes 236-283) +3. `docs/coding_guidelines.md` (crĂ©er section - plan lignes 287-324) + +**Commandes** : +```bash +# 1. Trouver tous les multi-mutex locks +grep -B3 -A3 "lock_guard" src/IntraIOManager.cpp + +# 2. Modifier les fichiers (utilise ton Ă©diteur) +nano src/IntraIOManager.cpp + +# 3. Build et test +cmake --build build +ctest --output-on-failure + +# 4. Validation TSan +cd build-tsan +cmake --build . +TSAN_OPTIONS="detect_deadlocks=1" ctest -V +``` + +**CritĂšres de succĂšs** : +- ✅ Tous les lock_guard multi-mutex remplacĂ©s +- ✅ Test `test_scoped_lock` passe +- ✅ TSan validation : 0 lock-order-inversion +- ✅ Documentation coding_guidelines.md créée + +--- + +#### Jour 8-10 : std::shared_mutex (6h) + +**Checklist - TopicTree** : +- [ ] Modifier `external/StillHammer/topictree/include/topictree/TopicTree.h` ligne 56 +- [ ] Changer `std::mutex` → `std::shared_mutex` +- [ ] Refactorer toutes les mĂ©thodes (voir plan lignes 406-478) + - `findSubscribers()` → `std::shared_lock` (READ) + - `registerSubscriber()` → `std::unique_lock` (WRITE) + - `unregisterSubscriber()` → `std::unique_lock` (WRITE) + - `clear()` → `std::unique_lock` (WRITE) + - `subscriberCount()` → `std::shared_lock` (READ) + +**Checklist - IntraIOManager** : +- [ ] Modifier `include/grove/IntraIOManager.h` +- [ ] Split `managerMutex` en 2 : `instancesMutex` (shared) + `statsMutex` (exclusive) +- [ ] Refactorer `src/IntraIOManager.cpp` (voir plan lignes 480-594) + - `getInstance()` → `std::shared_lock` (READ) + - `routeMessage()` → `std::shared_lock` pour lookup (CRITICAL!) + - `createInstance()` → `std::unique_lock` (WRITE) + - `removeInstance()` → `std::unique_lock` (WRITE) + +**Checklist - JsonDataTree (optionnel)** : +- [ ] Modifier `include/grove/JsonDataTree.h` et `src/JsonDataTree.cpp` +- [ ] Changer `std::mutex` → `std::shared_mutex` +- [ ] Refactorer mĂ©thodes (voir plan lignes 596-637) + +**Checklist - Benchmark** : +- [ ] CrĂ©er `tests/benchmarks/benchmark_shared_mutex.cpp` (plan lignes 641-688) +- [ ] Ajouter au CMakeLists.txt (plan lignes 690-698) +- [ ] Build : `cmake --build build` +- [ ] Run : `./build/tests/benchmark_shared_mutex --benchmark_min_time=3s` +- [ ] CrĂ©er rapport `docs/performance_reports/shared_mutex_results.md` (plan lignes 718-772) + +**Commandes** : +```bash +# 1. Modifier TopicTree.h +nano external/StillHammer/topictree/include/topictree/TopicTree.h + +# 2. Modifier IntraIOManager +nano include/grove/IntraIOManager.h +nano src/IntraIOManager.cpp + +# 3. Modifier JsonDataTree (optionnel) +nano include/grove/JsonDataTree.h +nano src/JsonDataTree.cpp + +# 4. CrĂ©er benchmark +mkdir -p tests/benchmarks +nano tests/benchmarks/benchmark_shared_mutex.cpp + +# 5. Modifier CMakeLists.txt pour benchmark +nano tests/CMakeLists.txt + +# 6. Build +cmake --build build -j$(nproc) + +# 7. Run tests +ctest --output-on-failure + +# 8. Validation TSan (CRITIQUE!) +cd build-tsan +cmake --build . +TSAN_OPTIONS="detect_deadlocks=1" ctest -V + +# 9. Run benchmark +cd ../build +./tests/benchmark_shared_mutex --benchmark_min_time=3s + +# 10. CrĂ©er rapport de performance +mkdir -p docs/performance_reports +nano docs/performance_reports/shared_mutex_results.md +``` + +**RĂ©sultats attendus du benchmark** : +``` +1 thread: Overhead nĂ©gligeable (~1%) +4 threads: Speedup 30-50x (mutex: 960ns → shared: 22ns) +8 threads: Speedup 150-250x (mutex: 7680ns → shared: 30ns) +``` + +**CritĂšres de succĂšs** : +- ✅ TopicTree utilise `shared_mutex` +- ✅ IntraIOManager split mutex (instances shared + stats exclusive) +- ✅ JsonDataTree utilise `shared_mutex` (optionnel) +- ✅ Benchmark créé et exĂ©cutĂ© +- ✅ Rapport de performance rempli (gain >50% dĂ©montrĂ©) +- ✅ TSan validation : 0 data race +- ✅ Tous les tests d'intĂ©gration passent + +--- + +## 🔧 Outils et Commandes Essentielles + +### Build Variants + +```bash +# Normal build +cmake -B build && cmake --build build + +# TSan build (dĂ©tection deadlock) +cmake -DGROVE_ENABLE_TSAN=ON -B build-tsan +cmake --build build-tsan + +# Helgrind build +cmake -DGROVE_ENABLE_HELGRIND=ON -B build +cmake --build build +cd build && make helgrind +``` + +### Tests + +```bash +# Tous les tests +ctest --output-on-failure + +# Test spĂ©cifique +./tests/test_13_cross_system + +# Avec TSan +TSAN_OPTIONS="detect_deadlocks=1 history_size=7" ./tests/test_13_cross_system + +# Avec timeout (si deadlock) +timeout 30 ./tests/test_13_cross_system || echo "DEADLOCK DETECTED" +``` + +### Debugging + +```bash +# Voir les mutex actifs +ps aux | grep test_13 +pstack # Si disponible + +# Logs TSan dĂ©taillĂ©s +TSAN_OPTIONS="detect_deadlocks=1 verbosity=2 log_path=tsan.log" ./test_13 + +# Analyser Helgrind +cat helgrind-full.log | grep -A10 "lock order" +``` + +--- + +## ⚠ PiĂšges Ă  Éviter + +### 1. Test 13 Deadlock (ligne 333) + +**ProblĂšme identifiĂ©** : +```cpp +// test_13_cross_system.cpp:331-333 +auto dataRootPtr = tree->getDataRootReadOnly(); // ❌ Raw pointer +dataRootPtr->setChild("player", ...); // ❌ Modification via read-only! +``` + +**Fix** : +```cpp +auto dataRoot = tree->getDataRoot(); // ✅ Writable unique_ptr +dataRoot->setChild("player", std::move(player5)); +``` + +### 2. TSan False Positives + +Si TSan rapporte des races sur : +- `spdlog` : Ajouter Ă  suppressions (lazy init benign) +- `std::atomic` : Normal, TSan ne comprend pas toujours +- Thread creation/join : Benign + +**Ne supprime PAS** : +- Lock-order-inversion : VRAI problĂšme! +- Data race sur variables non-atomic : VRAI problĂšme! + +### 3. Helgrind Lenteur + +- 10-50x slowdown est NORMAL +- Test complet peut prendre 1-2h +- Utilise `make helgrind-single` pour tester un seul test +- Patience! + +### 4. shared_mutex Performance + +- **VĂ©rifie le ratio Read/Write** : doit ĂȘtre >10:1 +- Si writes frĂ©quents (>30%), shared_mutex est PLUS LENT +- TopicTree : OK (100:1 ratio) +- IntraIOManager : OK (50:1 ratio) +- Stats counters : NON (1:1 ratio) → garde std::mutex + +--- + +## 📋 Checklist Finale de Validation + +Avant de considĂ©rer le plan terminĂ©, vĂ©rifie : + +### Phase 1 : DĂ©tection +- [ ] CMakeLists.txt avec options GROVE_ENABLE_TSAN et GROVE_ENABLE_HELGRIND +- [ ] Build TSan compile et tests passent (exit code 0) +- [ ] Build Helgrind targets fonctionnent (`make helgrind`) +- [ ] Fichier helgrind.supp avec suppressions appropriĂ©es +- [ ] 0 lock-order-inversion dans TSan output +- [ ] 0 data race dans TSan output +- [ ] Documentation TSan vs Helgrind (tableau comparatif) + +### Phase 2 : PrĂ©vention +- [ ] Tous les lock_guard multi-mutex → scoped_lock +- [ ] Test unitaire test_scoped_lock.cpp passe +- [ ] Documentation coding_guidelines.md mise Ă  jour +- [ ] TSan validation : 0 warning + +### Phase 3 : Optimisation +- [ ] TopicTree.h utilise std::shared_mutex +- [ ] IntraIOManager split instancesMutex (shared) + statsMutex (exclusive) +- [ ] JsonDataTree utilise std::shared_mutex (si applicable) +- [ ] Benchmark benchmark_shared_mutex créé et exĂ©cutĂ© +- [ ] Rapport de performance avec gains >50% documentĂ© +- [ ] TSan validation : 0 data race +- [ ] Tous les 13 tests d'intĂ©gration passent + +### Tests Critiques +- [ ] test_01_production_hotreload : PASS +- [ ] test_02_chaos_monkey : PASS +- [ ] test_03_stress_test : PASS (10 min) +- [ ] test_04_race_condition : PASS +- [ ] test_13_cross_system : PASS (fix deadlock ligne 333) + +### Documentation +- [ ] docs/tsan_findings.md créé +- [ ] docs/coding_guidelines.md mis Ă  jour +- [ ] docs/performance_reports/shared_mutex_results.md créé +- [ ] README.md des plans mis Ă  jour (statut 🟱) + +--- + +## 🎯 CritĂšres de SuccĂšs Final + +Le plan est **TERMINÉ** quand : + +1. ✅ **Build TSan** : `cmake -DGROVE_ENABLE_TSAN=ON -B build-tsan && cd build-tsan && TSAN_OPTIONS="detect_deadlocks=1" ctest` + - Exit code: 0 + - Output: "100% tests passed, 0 tests failed" + - Aucun warning TSan + +2. ✅ **Build Helgrind** : `cd build && make helgrind` + - Exit code: 0 + - helgrind-full.log propre (aprĂšs suppressions) + +3. ✅ **Tests d'intĂ©gration** : `ctest --output-on-failure` + - 13/13 tests passent + - Dont test_13_cross_system (fix deadlock) + +4. ✅ **Benchmark** : `./tests/benchmark_shared_mutex` + - Speedup >50% dĂ©montrĂ© (4 threads) + - Rapport de performance documentĂ© + +5. ✅ **Code Review** : + - Aucun lock_guard multi-mutex restant + - Tous les read-heavy locks sont shared_lock + - Tous les write locks sont unique_lock + +--- + +## 📞 En Cas de ProblĂšme + +### Test 13 deadlock toujours prĂ©sent + +```bash +# 1. Identifier oĂč ça bloque +timeout 10 ./tests/test_13_cross_system +ps aux | grep test_13 +pstack # Voir les stacks + +# 2. VĂ©rifier le fix ligne 333 +grep -n "getDataRootReadOnly" tests/integration/test_13_cross_system.cpp +# Doit utiliser getDataRoot() au lieu de getDataRootReadOnly() + +# 3. Rebuild et retest +cmake --build build +./tests/test_13_cross_system +``` + +### TSan trouve encore des lock-order-inversions + +```bash +# 1. Voir exactement oĂč +TSAN_OPTIONS="detect_deadlocks=1 history_size=7 second_deadlock_stack=1" ./problematic_test + +# 2. Identifier les mutexes (M1, M2) +# 3. Utiliser scoped_lock(M1, M2) partout +# 4. Rebuild et retest +``` + +### Helgrind timeout + +```bash +# 1. Run un seul test +make helgrind-single + +# 2. Si trop lent, skip Helgrind +# TSan est suffisant pour validation +``` + +### Benchmark ne montre pas de gain + +```bash +# 1. VĂ©rifier le ratio read/write +# Si writes > 30%, shared_mutex est inutile + +# 2. VĂ©rifier que tu as bien utilisĂ© shared_lock pour les reads +grep "shared_lock" src/IntraIOManager.cpp + +# 3. Run benchmark avec plus de threads +./benchmark_shared_mutex --benchmark_filter=.*MultiThread.*/threads:8 +``` + +--- + +## 📝 Rapport Final Ă  Produire + +Quand tout est terminĂ©, crĂ©e : `docs/plans/REPORT_deadlock_plan_completed.md` + +**Template** : +```markdown +# Rapport : Plan Deadlock Detection & Prevention - TERMINÉ + +**Date de complĂ©tion** : YYYY-MM-DD +**DurĂ©e rĂ©elle** : Xh (estimĂ©: 15h) +**Statut** : ✅ TERMINÉ + +## RĂ©sumĂ© + +[1-2 paragraphes rĂ©sumant ce qui a Ă©tĂ© fait] + +## Phase 1 : DĂ©tection (5h) + +- ✅ ThreadSanitizer intĂ©grĂ© (2h rĂ©elles) + - Warnings TSan trouvĂ©s : X + - Fixes appliquĂ©s : Y +- ✅ Helgrind intĂ©grĂ© (3h rĂ©elles) + - Suppressions ajoutĂ©es : Z + +## Phase 2 : PrĂ©vention (4h) + +- ✅ scoped_lock refactoring (4h rĂ©elles) + - Fichiers modifiĂ©s : X + - Lock_guard → scoped_lock : Y endroits + +## Phase 3 : Optimisation (6h) + +- ✅ shared_mutex implĂ©mentĂ© (6h rĂ©elles) + - TopicTree : ✅ + - IntraIOManager : ✅ + - JsonDataTree : ✅/❌ + - Speedup mesurĂ© : Xx (4 threads), Yy (8 threads) + +## Tests Validation + +| Test | Avant | AprĂšs | Statut | +|------|-------|-------|--------| +| test_01 | ✅ | ✅ | OK | +| test_02 | ✅ | ✅ | OK | +| ... | ... | ... | ... | +| test_13 | ❌ Deadlock | ✅ | FIXED | + +## Benchmark Results + +[Copier les rĂ©sultats du benchmark] + +## ProblĂšmes RencontrĂ©s + +1. [ProblĂšme 1 et solution] +2. [ProblĂšme 2 et solution] + +## Leçons Apprises + +[Ce qui a Ă©tĂ© appris pendant l'implĂ©mentation] + +## Recommandations Futures + +1. [AmĂ©lioration 1] +2. [AmĂ©lioration 2] +``` + +--- + +## 🚀 Let's Go! + +Tu as maintenant **TOUT** ce qu'il faut : +- ✅ Plan dĂ©taillĂ© (30KB de specs) +- ✅ Code AVANT/APRÈS ligne par ligne +- ✅ Commandes exactes +- ✅ Tests de validation +- ✅ Debugging tips +- ✅ Checklist complĂšte + +**Commence par la Phase 1.1 (ThreadSanitizer - 2h)** : +```bash +# 1. Ouvre le plan +cat docs/plans/PLAN_deadlock_detection_prevention.md + +# 2. Copie le code CMake (lignes 21-35) +nano CMakeLists.txt + +# 3. Build +cmake -DGROVE_ENABLE_TSAN=ON -B build-tsan +cmake --build build-tsan + +# 4. Test +cd build-tsan +TSAN_OPTIONS="detect_deadlocks=1" ctest -V + +# 5. Fix tous les warnings +# 6. Re-test jusqu'Ă  0 warning +``` + +**Bonne chance ! 🎯** + +--- + +**Auteur** : Claude Code +**Version** : 1.0 +**Pour** : Successeur/ImplĂ©menteur du plan deadlock +**Date** : 2025-01-21