Skip to content

CGlobalCategoryId holds a raw back-pointer to the mapper's categorizer key, which dangles after clone() #3144

Description

@edsavage

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<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:

  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<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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions