You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
CGlobalCategoryId holds a raw back-pointer to the mapper's categorizer key, which dangles after clone() #3144
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.
clone() → std::make_shared<CPerPartitionCategoryIdMapper>(*this) (default copy of m_Mappings).
map() constructs CGlobalCategoryIds 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:
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).
Store the categorizer key in CGlobalCategoryId by shared ownership (e.g. std::shared_ptr<const std::string>) 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.
Activity