CGlobalCategoryId holds a raw back-pointer to the mapper's categorizer key, which dangles after clone()
- Vorherrschende Sprache
- C++
- Sterne
- 157
- Forks
- 67
- Ø Merge
- 17 Std. 52 Min.
- Gemergte PRs (30 T.)
- 20
Beschreibung
### Summary
`CGlobalCategoryId` stores a **raw pointer** to the owning mapper's categorizer-key string (`m_CategorizerKey = &categorizerKey`). Because `CPerPartitionCategoryIdMapper::clone()` performs a default (shallow) copy of its `m_Mappings` vector, the cloned `CGlobalCategoryId` entries still point at the **original** mapper's key string rather than the clone's. If the original mapper is destroyed while the clone is still alive, those pointers dangle.
This is a latent lifetime issue, distinct from the state-corruption crash class tracked in #2875. Splitting it out here so it isn't lost.
### Where
- `lib/api/CGlobalCategoryId.cc` — the `(int, const std::string&, CLocalCategoryId)` constructor stores `m_CategorizerKey = &categorizerKey`.
- `lib/api/CPerPartitionCategoryIdMapper.cc`
- `clone()` → `std::make_shared(*this)` (default copy of `m_Mappings`).
- `map()` constructs `CGlobalCategoryId`s bound to `m_CategorizerKey`.
### Impact
- **Persistence is unaffected**: `CPerPartitionCategoryIdMapper::acceptPersistInserter` only reads `globalCategoryId.globalId()`; it never dereferences the key back-pointer.
- The dangling read is only reachable via `CGlobalCategoryId::print()` / `operator<<` (i.e. logging/diagnostics) on a cloned mapper whose original has since been destroyed. In practice `clone()` is used for background persistence snapshots, so the window is narrow — but it is undefined behaviour.
### Suggested fix
Make `CGlobalCategoryId` not depend on a pointer whose lifetime it doesn't control. Options:
1. Have the copy path re-bind cloned mappings to the clone's own `m_CategorizerKey` (e.g. a user-provided copy constructor in `CPerPartitionCategoryIdMapper` that rewrites each mapping's key reference).
2. Store the categorizer key in `CGlobalCategoryId` by shared ownership (e.g. `std::shared_ptr`) so clones stay valid independent of the original mapper.
Option 1 keeps `CGlobalCategoryId` lightweight; option 2 is more robust but adds an allocation per key.
### Notes
Found during the deep dive accompanying the #2875 restore-hardening work (PR #3143). No crash has been attributed to this yet; filing pre-emptively.
Beitragsleitfaden
Rechercherichtung
Lesen Sie lib/api/CGlobalCategoryId.cc und lib/api/CPerPartitionCategoryIdMapper.cc, wobei Sie sich auf den Konstruktor, clone(), map() und acceptPersistInserter() konzentrieren. Verfolgen Sie, wie geklonte Mappings den categorizer-key pointer beibehalten, und prüfen Sie anschließend vorhandene Copy- und Persistenztests auf ihre Abdeckung. Als erledigt gilt die Aufgabe, wenn ein Clone noch sicher ausgegeben werden kann, nachdem der ursprüngliche Mapper zerstört wurde, ohne das Persistenzverhalten zu ändern.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Bewertung
- Tech-Stack
- cpp
- Bereich
- backend
- Issue-Typ
- Bug
- Schwierigkeit
- 4/5
- Geschätzter Aufwand
- 3-5 Tage
- Aktivitätsstatus
- Ruhig
- Klarheit
- Größtenteils klar
- Anfängerfreundlichkeit
- 55/100