CGlobalCategoryId holds a raw back-pointer to the mapper's categorizer key, which dangles after clone()
- Dominant language
- C++
- Stars
- 157
- Forks
- 67
- Avg merge
- 12h 48m
- Merged PRs (30d)
- 16
Description
### 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.
Contributor guide
Assessment
This issue has not been assessed yet.