Skip to content
Snippets Groups Projects
  • Richard Mudgett's avatar
    b97dbd72
    DEBUG_THREADS: Fix regression and lock tracking initialization problems. · b97dbd72
    Richard Mudgett authored
    This patch started with David Lee's patch at
    https://reviewboard.asterisk.org/r/2826/ and includes a regression fix
    introduced by the ASTERISK-22455 patch.
    
    The initialization of a mutex's lock tracking structure was not protected
    in a critical section.  This is fine for any mutex that is explicitly
    initialized, but a static mutex may have its lock tracking double
    initialized if multiple threads attempt the first lock simultaneously.
    
    * Added a global mutex to properly serialize initialization of the lock
    tracking structure.  The painful global lock can be mitigated by adding a
    double checked lock flag as discussed on the original review request.
    
    * Defer lock tracking initialization until first use.
    
    * Don't be "helpful" and initialize an uninitialized lock when
    DEBUG_THREADS is enabled.  Debug code is not supposed to fix or change
    normal code behavior.  We don't need a lock initialization race that would
    force a re-setup of lock tracking.  Lock tracking already handles
    initialization on first use.
    
    * Properly handle allocation failures of the lock tracking structure.
    
    * No need to initialize tracking data in __ast_pthread_mutex_destroy()
    just to turn around and destroy it.
    
    
    The regression introduced by ASTERISK-22455 is the result of manipulating
    a pthread_mutex_t struct outside of the pthread library code.  The
    pthread_mutex_t struct seems to have a global linked list pointer member
    that can get changed by other threads.  Therefore, saving and restoring
    the contents of a pthread_mutex_t struct is a bad thing.
    
    Thanks to Thomas Airmont for finding this obscure regression.
    
    * Don't overwrite the struct ast_lock_track.reentr_mutex member to restore
    tracking data in __ast_cond_wait() and __ast_cond_timedwait().  The
    pthread_mutex_t struct must be treated as a read-only opaque variable.
    
    
    Miscellaneous other items fixed by this patch:
    
    * Match ast_suspend_lock_info() with ast_restore_lock_info() in
    __ast_cond_timedwait().
    
    * Made some uninitialized lock sanity checks return EINVAL and try a
    DO_THREAD_CRASH.
    
    * Fix bad canlog initialization expressions.
    
    ASTERISK-24614 #close
    Reported by: Thomas Airmont
    
    Review: https://reviewboard.asterisk.org/r/4247/
    Review: https://reviewboard.asterisk.org/r/2826/
    
    
    git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/11@429539 65c4cc65-6c06-0410-ace0-fbb531ad65f3
    b97dbd72
    History
    DEBUG_THREADS: Fix regression and lock tracking initialization problems.
    Richard Mudgett authored
    This patch started with David Lee's patch at
    https://reviewboard.asterisk.org/r/2826/ and includes a regression fix
    introduced by the ASTERISK-22455 patch.
    
    The initialization of a mutex's lock tracking structure was not protected
    in a critical section.  This is fine for any mutex that is explicitly
    initialized, but a static mutex may have its lock tracking double
    initialized if multiple threads attempt the first lock simultaneously.
    
    * Added a global mutex to properly serialize initialization of the lock
    tracking structure.  The painful global lock can be mitigated by adding a
    double checked lock flag as discussed on the original review request.
    
    * Defer lock tracking initialization until first use.
    
    * Don't be "helpful" and initialize an uninitialized lock when
    DEBUG_THREADS is enabled.  Debug code is not supposed to fix or change
    normal code behavior.  We don't need a lock initialization race that would
    force a re-setup of lock tracking.  Lock tracking already handles
    initialization on first use.
    
    * Properly handle allocation failures of the lock tracking structure.
    
    * No need to initialize tracking data in __ast_pthread_mutex_destroy()
    just to turn around and destroy it.
    
    
    The regression introduced by ASTERISK-22455 is the result of manipulating
    a pthread_mutex_t struct outside of the pthread library code.  The
    pthread_mutex_t struct seems to have a global linked list pointer member
    that can get changed by other threads.  Therefore, saving and restoring
    the contents of a pthread_mutex_t struct is a bad thing.
    
    Thanks to Thomas Airmont for finding this obscure regression.
    
    * Don't overwrite the struct ast_lock_track.reentr_mutex member to restore
    tracking data in __ast_cond_wait() and __ast_cond_timedwait().  The
    pthread_mutex_t struct must be treated as a read-only opaque variable.
    
    
    Miscellaneous other items fixed by this patch:
    
    * Match ast_suspend_lock_info() with ast_restore_lock_info() in
    __ast_cond_timedwait().
    
    * Made some uninitialized lock sanity checks return EINVAL and try a
    DO_THREAD_CRASH.
    
    * Fix bad canlog initialization expressions.
    
    ASTERISK-24614 #close
    Reported by: Thomas Airmont
    
    Review: https://reviewboard.asterisk.org/r/4247/
    Review: https://reviewboard.asterisk.org/r/2826/
    
    
    git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/11@429539 65c4cc65-6c06-0410-ace0-fbb531ad65f3