Skip to content
Snippets Groups Projects
  • Nat Goodspeed's avatar
    9d5d257c
    DRTVWR-476, SL-12204: Fix crash in Marketplace Listings. · 9d5d257c
    Nat Goodspeed authored
    The observed crash was due to sharing a stateful global resource (the global
    LLMessageSystem instance) between different tasks. Specifically, a coroutine
    sets its mMessageReader one way, expecting that value to persist until it's
    done with message parsing, but another coroutine sneaks in at a suspension
    point and sets it differently.
    
    Introduce LockMessageReader and LockMessageChecker classes, which must be
    instantiated by a consumer of the resource. The constructor of each locks a
    coroutine-aware mutex, so that for the lifetime of the lock object no other
    coroutine can instantiate another.
    
    Refactor the code so that LLMessageSystem::mMessageReader can only be modified
    by LockMessageReader, not by direct assignment. mMessageReader is now an
    instance of LLMessageReaderPointer, which supports dereferencing and
    comparison but not assignment. Only LockMessageReader can change its value.
    
    LockMessageReader addresses the use case in which the specific mMessageReader
    value need only persist for the duration of a single method call. Add an
    instance in LLMessageHandlerBridge::post().
    
    LockMessageChecker is a subclass of LockMessageReader: both lock the same
    mutex. LockMessageChecker addresses the use case in which the specific
    mMessageReader value must persist across multiple method calls. Modify the
    methods in question to require a LockMessageChecker instance. Provide
    LockMessageChecker forwarding methods to facilitate calling the underlying
    LLMessageSystem methods via the LockMessageChecker instance.
    
    Add LockMessageChecker instances to LLAppViewer::idleNetwork(), a couple cases
    in idle_startup() and LLMessageSystem::establishBidirectionalTrust().
    9d5d257c
    History
    DRTVWR-476, SL-12204: Fix crash in Marketplace Listings.
    Nat Goodspeed authored
    The observed crash was due to sharing a stateful global resource (the global
    LLMessageSystem instance) between different tasks. Specifically, a coroutine
    sets its mMessageReader one way, expecting that value to persist until it's
    done with message parsing, but another coroutine sneaks in at a suspension
    point and sets it differently.
    
    Introduce LockMessageReader and LockMessageChecker classes, which must be
    instantiated by a consumer of the resource. The constructor of each locks a
    coroutine-aware mutex, so that for the lifetime of the lock object no other
    coroutine can instantiate another.
    
    Refactor the code so that LLMessageSystem::mMessageReader can only be modified
    by LockMessageReader, not by direct assignment. mMessageReader is now an
    instance of LLMessageReaderPointer, which supports dereferencing and
    comparison but not assignment. Only LockMessageReader can change its value.
    
    LockMessageReader addresses the use case in which the specific mMessageReader
    value need only persist for the duration of a single method call. Add an
    instance in LLMessageHandlerBridge::post().
    
    LockMessageChecker is a subclass of LockMessageReader: both lock the same
    mutex. LockMessageChecker addresses the use case in which the specific
    mMessageReader value must persist across multiple method calls. Modify the
    methods in question to require a LockMessageChecker instance. Provide
    LockMessageChecker forwarding methods to facilitate calling the underlying
    LLMessageSystem methods via the LockMessageChecker instance.
    
    Add LockMessageChecker instances to LLAppViewer::idleNetwork(), a couple cases
    in idle_startup() and LLMessageSystem::establishBidirectionalTrust().
Code owners
Assign users and groups as approvers for specific file changes. Learn more.