Skip to content
Snippets Groups Projects
  • Nat Goodspeed's avatar
    9d5b8976
    DRTVWR-494: Defend LLInstanceTracker against multi-thread usage. · 9d5b8976
    Nat Goodspeed authored
    The previous implementation went to some effort to crash if anyone attempted
    to create or destroy an LLInstanceTracker subclass instance during traversal.
    That restriction is manageable within a single thread, but becomes unworkable
    if it's possible that a given subclass might be used on more than one thread.
    
    Remove LLInstanceTracker::instance_iter, beginInstances(), endInstances(),
    also key_iter, beginKeys() and endKeys(). Instead, introduce key_snapshot()
    and instance_snapshot(), the only means of iterating over LLInstanceTracker
    instances. (These are intended to resemble functions, but in fact the current
    implementation simply presents the classes.) Iterating over a captured
    snapshot defends against container modifications during traversal. The term
    'snapshot' reminds the coder that a new instance created during traversal will
    not be considered. To defend against instance deletion during traversal, a
    snapshot stores std::weak_ptrs which it lazily dereferences, skipping on the
    fly any that have expired.
    
    Dereferencing instance_snapshot::iterator gets you a reference rather than a
    pointer. Because some use cases want to delete all existing instances, add an
    instance_snapshot::deleteAll() method that extracts the pointer. Those cases
    used to require explicitly copying instance pointers into a separate
    container; instance_snapshot() now takes care of that. It remains the caller's
    responsibility to ensure that all instances of that LLInstanceTracker subclass
    were allocated on the heap.
    
    Replace unkeyed static LLInstanceTracker::getInstance(T*) -- which returned
    nullptr if that instance had been destroyed -- with new getWeak() method
    returning std::weak_ptr<T>. Caller must detect expiration of that weak_ptr.
    
    Adjust tests accordingly.
    
    Use of std::weak_ptr to detect expired instances requires engaging
    std::shared_ptr in the constructor. We now store shared_ptrs in the static
    containers (std::map for keyed, std::set for unkeyed).
    
    Make LLInstanceTrackerBase a template parameterized on the type of the static
    data it manages. For that reason, hoist static data class declarations out of
    the class definitions to an LLInstanceTrackerStuff namespace.
    
    Remove the static atomic sIterationNestDepth and its methods incrementDepth(),
    decrementDepth() and getDepth(), since they were used only to forbid creation
    and destruction during traversal.
    
    Add a std::mutex to static data. Introduce an internal LockStatic class that
    locks the mutex while providing a pointer to static data, making that the only
    way to access the static data.
    
    The LLINSTANCETRACKER_DTOR_NOEXCEPT macro goes away because we no longer
    expect ~LLInstanceTracker() to throw an exception in test programs.
    That affects LLTrace::StatBase as well as LLInstanceTracker itself.
    
    Adapt consumers to the new LLInstanceTracker API.
    9d5b8976
    History
    DRTVWR-494: Defend LLInstanceTracker against multi-thread usage.
    Nat Goodspeed authored
    The previous implementation went to some effort to crash if anyone attempted
    to create or destroy an LLInstanceTracker subclass instance during traversal.
    That restriction is manageable within a single thread, but becomes unworkable
    if it's possible that a given subclass might be used on more than one thread.
    
    Remove LLInstanceTracker::instance_iter, beginInstances(), endInstances(),
    also key_iter, beginKeys() and endKeys(). Instead, introduce key_snapshot()
    and instance_snapshot(), the only means of iterating over LLInstanceTracker
    instances. (These are intended to resemble functions, but in fact the current
    implementation simply presents the classes.) Iterating over a captured
    snapshot defends against container modifications during traversal. The term
    'snapshot' reminds the coder that a new instance created during traversal will
    not be considered. To defend against instance deletion during traversal, a
    snapshot stores std::weak_ptrs which it lazily dereferences, skipping on the
    fly any that have expired.
    
    Dereferencing instance_snapshot::iterator gets you a reference rather than a
    pointer. Because some use cases want to delete all existing instances, add an
    instance_snapshot::deleteAll() method that extracts the pointer. Those cases
    used to require explicitly copying instance pointers into a separate
    container; instance_snapshot() now takes care of that. It remains the caller's
    responsibility to ensure that all instances of that LLInstanceTracker subclass
    were allocated on the heap.
    
    Replace unkeyed static LLInstanceTracker::getInstance(T*) -- which returned
    nullptr if that instance had been destroyed -- with new getWeak() method
    returning std::weak_ptr<T>. Caller must detect expiration of that weak_ptr.
    
    Adjust tests accordingly.
    
    Use of std::weak_ptr to detect expired instances requires engaging
    std::shared_ptr in the constructor. We now store shared_ptrs in the static
    containers (std::map for keyed, std::set for unkeyed).
    
    Make LLInstanceTrackerBase a template parameterized on the type of the static
    data it manages. For that reason, hoist static data class declarations out of
    the class definitions to an LLInstanceTrackerStuff namespace.
    
    Remove the static atomic sIterationNestDepth and its methods incrementDepth(),
    decrementDepth() and getDepth(), since they were used only to forbid creation
    and destruction during traversal.
    
    Add a std::mutex to static data. Introduce an internal LockStatic class that
    locks the mutex while providing a pointer to static data, making that the only
    way to access the static data.
    
    The LLINSTANCETRACKER_DTOR_NOEXCEPT macro goes away because we no longer
    expect ~LLInstanceTracker() to throw an exception in test programs.
    That affects LLTrace::StatBase as well as LLInstanceTracker itself.
    
    Adapt consumers to the new LLInstanceTracker API.
Code owners
Assign users and groups as approvers for specific file changes. Learn more.