Skip to content
Snippets Groups Projects
  • Nat Goodspeed's avatar
    a6f5e55d
    DRTVWR-494: Dispatch all LLSingleton construction to the main thread. · a6f5e55d
    Nat Goodspeed authored
    Given the viewer's mutually-dependent LLSingletons, given that different
    threads might simultaneously request different LLSingletons from such a chain
    of circular dependencies, the key to avoiding deadlock is to serialize all
    LLSingleton construction on one thread: the main thread. Add comments to
    LLSingleton::getInstance() explaining the problem and the solution.
    
    Recast LLSingleton's static SingletonData to use LockStatic. Instead of using
    Locker, and simply trusting that every reference to sData is within the
    dynamic scope of a Locker instance, LockStatic enforces that: you can only
    access SingletonData members via LockStatic.
    
    Reorganize the switch in getInstance() to group the CONSTRUCTING error, the
    INITIALIZING/INITIALIZED success case, and the DELETED/UNINITIALIZED
    construction case.
    
    When [re]constructing an instance, on the main thread, retain the lock and
    call constructSingleton() (and capture_dependency()) directly.
    
    On a secondary thread, unlock LockStatic and use LLMainThreadTask::dispatch()
    to call getInstance() on the main thread. Since we might end up enqueuing
    multiple such tasks, it's important to let getInstance() notice when the
    instance has already been constructed and simply return the existing pointer.
    
    Add loginfos() method, sibling to logerrs(), logwarns() and logdebugs().
    Produce loginfos() messages when dispatching to the main thread, when actually
    running on the main thread and when resuming the suspended requesting thread.
    
    Make deleteSingleton() manage all associated state, instead of delegating some
    of that work to ~LLSingleton(). Now, within LockStatic, extract the instance
    pointer and set state to DELETED; that lets subsequent code, which retains the
    only remaining pointer to the instance, remove the master-list entry, call the
    subclass cleanupSingleton() and destructor without needing to hold the lock.
    
    In fact, entirely remove ~LLSingleton().
    
    Import LLSingletonBase::cleanup_() method to wrap the call to subclass
    cleanupSingleton() in try/catch.
    
    Remove cleanupAll() calls from llsingleton_test.cpp, and reorder the success
    cases to reflect the fact that T::cleanupSingleton() is called immediately
    before ~T() for each distinct LLSingleton subclass T.
    
    When getInstance() on a secondary thread dispatches to the main thread, it
    necessarily unlocks its LockStatic lock. But an LLSingleton dependency chain
    strongly depends on the function stack on which getInstance() is invoked --
    the task dispatched to the main thread doesn't know the dependencies tracked
    on the requesting thread stack. So, once the main thread delivers the instance
    pointer, the requesting thread captures its own dependencies for that
    instance.
    
    Back in the requesting thread, obtaining the current EInitState to pass to
    capture_dependencies() would have required relocking LockStatic. Instead, I've
    convinced myself that (a) capture_dependencies() only wanted to know
    EInitState to produce an error for CONSTRUCTING, and (b) in CONSTRUCTING
    state, we never get as far as capture_dependencies() because getInstance()
    produces an error first.
    
    Eliminate the EInitState parameter from all capture_dependencies() methods.
    Remove the LLSingletonBase::capture_dependency() stanza that tested
    EInitState. Make the capture_dependencies() variants that accepted LockStatic
    instead accept LLSingletonBase*. That lets getInstance(), in the
    LLMainThreadTask case, pass the newly-returned instance pointer.
    
    For symmetry, make pop_initializing() accept LLSingletonBase* as well, instead
    of accepting LockStatic and extracting mInstance.
    a6f5e55d
    History
    DRTVWR-494: Dispatch all LLSingleton construction to the main thread.
    Nat Goodspeed authored
    Given the viewer's mutually-dependent LLSingletons, given that different
    threads might simultaneously request different LLSingletons from such a chain
    of circular dependencies, the key to avoiding deadlock is to serialize all
    LLSingleton construction on one thread: the main thread. Add comments to
    LLSingleton::getInstance() explaining the problem and the solution.
    
    Recast LLSingleton's static SingletonData to use LockStatic. Instead of using
    Locker, and simply trusting that every reference to sData is within the
    dynamic scope of a Locker instance, LockStatic enforces that: you can only
    access SingletonData members via LockStatic.
    
    Reorganize the switch in getInstance() to group the CONSTRUCTING error, the
    INITIALIZING/INITIALIZED success case, and the DELETED/UNINITIALIZED
    construction case.
    
    When [re]constructing an instance, on the main thread, retain the lock and
    call constructSingleton() (and capture_dependency()) directly.
    
    On a secondary thread, unlock LockStatic and use LLMainThreadTask::dispatch()
    to call getInstance() on the main thread. Since we might end up enqueuing
    multiple such tasks, it's important to let getInstance() notice when the
    instance has already been constructed and simply return the existing pointer.
    
    Add loginfos() method, sibling to logerrs(), logwarns() and logdebugs().
    Produce loginfos() messages when dispatching to the main thread, when actually
    running on the main thread and when resuming the suspended requesting thread.
    
    Make deleteSingleton() manage all associated state, instead of delegating some
    of that work to ~LLSingleton(). Now, within LockStatic, extract the instance
    pointer and set state to DELETED; that lets subsequent code, which retains the
    only remaining pointer to the instance, remove the master-list entry, call the
    subclass cleanupSingleton() and destructor without needing to hold the lock.
    
    In fact, entirely remove ~LLSingleton().
    
    Import LLSingletonBase::cleanup_() method to wrap the call to subclass
    cleanupSingleton() in try/catch.
    
    Remove cleanupAll() calls from llsingleton_test.cpp, and reorder the success
    cases to reflect the fact that T::cleanupSingleton() is called immediately
    before ~T() for each distinct LLSingleton subclass T.
    
    When getInstance() on a secondary thread dispatches to the main thread, it
    necessarily unlocks its LockStatic lock. But an LLSingleton dependency chain
    strongly depends on the function stack on which getInstance() is invoked --
    the task dispatched to the main thread doesn't know the dependencies tracked
    on the requesting thread stack. So, once the main thread delivers the instance
    pointer, the requesting thread captures its own dependencies for that
    instance.
    
    Back in the requesting thread, obtaining the current EInitState to pass to
    capture_dependencies() would have required relocking LockStatic. Instead, I've
    convinced myself that (a) capture_dependencies() only wanted to know
    EInitState to produce an error for CONSTRUCTING, and (b) in CONSTRUCTING
    state, we never get as far as capture_dependencies() because getInstance()
    produces an error first.
    
    Eliminate the EInitState parameter from all capture_dependencies() methods.
    Remove the LLSingletonBase::capture_dependency() stanza that tested
    EInitState. Make the capture_dependencies() variants that accepted LockStatic
    instead accept LLSingletonBase*. That lets getInstance(), in the
    LLMainThreadTask case, pass the newly-returned instance pointer.
    
    For symmetry, make pop_initializing() accept LLSingletonBase* as well, instead
    of accepting LockStatic and extracting mInstance.
Code owners
Assign users and groups as approvers for specific file changes. Learn more.