VirtualBox

Changeset 25408 in vbox


Ignore:
Timestamp:
Dec 15, 2009 3:04:04 PM (15 years ago)
Author:
vboxsync
Message:

Main/Locking: fix regression in AutoMultiWriteLock that caused confusion in how many times locks needed to be unlocked; this caused lock validation to fail in restoreSnapshot(); also don't require unlocking to happen in exactly the reverse order of locking; documentation

Location:
trunk/src/VBox/Main
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Main/AutoLock.cpp

    r25368 r25408  
    11/** @file
    22 *
    3  * AutoWriteLock/AutoReadLock: smart R/W semaphore wrappers
     3 * Automatic locks, implementation
    44 */
    55
    66/*
    7  * Copyright (C) 2006-2000 Sun Microsystems, Inc.
     7 * Copyright (C) 2006-2009 Sun Microsystems, Inc.
    88 *
    99 * This file is part of VirtualBox Open Source Edition (OSE), as
    … …  
    3939#include <iprt/path.h>
    4040
     41#include <VBox/com/string.h>
     42
    4143#include <vector>
    4244#include <list>
    … …  
    4749////////////////////////////////////////////////////////////////////////////////
    4850//
    49 // Global variables
     51// Per-thread stacks for locking validation
    5052//
    5153////////////////////////////////////////////////////////////////////////////////
    … …  
    8284
    8385typedef std::list<LockStackItem> LockHandlesList;
     86
     87/**
     88 * LockingStack class. One of these gets created for each thread
     89 * that calls lock/unlock methods, and a pointer to this is
     90 * stored in thread-local storage.
     91 */
    8492struct LockingStack
    8593{
    … …  
    97105
    98106    LockHandlesList     ll;
     107            // first item (front) is newest, last item (back) is oldest; I'd do it
     108            // the other way round but there is no implementation for erase(reverse_iterator)
     109            // which I'd need otherwise
    99110    size_t              c;
    100111};
    101112
     113/**
     114 * Global helper that looks up the LockingStack structure for the
     115 * current thread in thread-local storage, or creates one on the
     116 * thread's first call.
     117 */
    102118LockingStack* getThreadLocalLockingStack()
    103119{
    … …  
    120136    return pStack;
    121137}
     138
     139void dumpThreadLocalLockingStack(LockingStack *pStack)
     140{
     141    uint32_t c = 0;
     142    for (LockHandlesList::iterator it = pStack->ll.begin();
     143         it != pStack->ll.end();
     144         ++it)
     145    {
     146        LockStackItem lsi = *it;
     147        LogFlow(("LOCKVAL:   lock %d under top is [%s] locked by %s (%s:%u)\n", c, lsi.pLock->describe(), lsi.pcszFunction, lsi.pcszFile, lsi.uLine));
     148        ++c;
     149    }
     150}
     151
    122152#endif
    123153
    … …  
    129159
    130160#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     161
     162/**
     163 * If the lock validator is enabled, this gets called from the
     164 * lock methods to push a LockStackItem onto the thread-local
     165 * locking stack for tracing.
     166 */
    131167void LockHandle::validateLock(LOCKVAL_SRC_POS_DECL)
    132168{
    … …  
    134170    LockingStack *pStack = getThreadLocalLockingStack();
    135171    LockStackItem lsi(this, RT_SRC_POS_ARGS);
    136     pStack->ll.push_back(lsi);
     172    pStack->ll.push_front(lsi);
    137173    ++pStack->c;
    138174
    139     LogFlow(("LOCKVAL: lock from %s (%s:%u), new count: %RI32\n", lsi.pcszFunction, lsi.pcszFile, lsi.uLine, (uint32_t)pStack->c));
    140 }
    141 
     175    LogFlow(("LOCKVAL [%s]: lock from %s (%s:%u), new count: %RI32\n", describe(), lsi.pcszFunction, lsi.pcszFile, lsi.uLine, (uint32_t)pStack->c));
     176}
     177
     178/**
     179 * If the lock validator is enabled, this gets called from the
     180 * unlock methods to validate the unlock request. This pops the
     181 * LockStackItem from the thread-local locking stack.
     182 */
    142183void LockHandle::validateUnlock()
    143184{
    … …  
    145186    LockingStack *pStack = getThreadLocalLockingStack();
    146187
    147     LogFlow(("LOCKVAL: unlock, old count: %RI32\n", (uint32_t)pStack->c));
    148 
    149188    AssertMsg(pStack->c == pStack->ll.size(), ("Locking size mismatch"));
    150189    AssertMsg(pStack->c > 0, ("Locking stack is empty when it should have current LockHandle on top"));
    151190
    152191    // validate that "this" is the top item on the stack
    153     LockStackItem &lsiTop = pStack->ll.back();
     192    LockStackItem &lsiTop = pStack->ll.front();
    154193    if (lsiTop.pLock != this)
    155194    {
    156         // violation of unlocking order: "this" was not the last to be locked on this thread,
     195        // "this" was not the last to be locked on this thread;
    157196        // see if it's somewhere deep under the locks
     197        dumpThreadLocalLockingStack(pStack);
     198
    158199        bool fFound;
    159200        uint32_t c = 0;
    160201        for (LockHandlesList::iterator it = pStack->ll.begin();
    161202             it != pStack->ll.end();
    162              ++it, ++c)
     203             ++it)
    163204        {
    164205            LockStackItem &lsiThis = *it;
    165206            if (lsiThis.pLock == this)
    166207            {
    167                 AssertMsgFailed(("Unlocking order violation: unlock attempted for LockHandle which is %d items under the top item\n", c));
     208                LogFlow(("LOCKVAL [%s]: unlock, stack item was %d items under the top item, corresponsing lock was at %s (%s:%u)\n", describe(), c, lsiThis.pcszFunction, lsiThis.pcszFile, lsiThis.uLine));
    168209                pStack->ll.erase(it);
    169210                fFound = true;
    170211                break;
    171212            }
     213            ++c;
    172214        }
    173215
    174216        if (!fFound)
     217        {
     218            LogFlow(("LOCKVAL [%s]: unlock, stack item not found!\n", describe()));
    175219            AssertMsgFailed(("Locking stack does not contain current LockHandle at all\n"));
     220        }
    176221    }
    177222    else
    178         pStack->ll.pop_back();
     223    {
     224        pStack->ll.pop_front();
     225        LogFlow(("LOCKVAL [%s]: unlock, stack item was on top, old count: %RI32\n", describe(), (uint32_t)pStack->c));
     226    }
     227
    179228    --pStack->c;
    180229}
     230
    181231#endif // VBOX_WITH_DEBUG_LOCK_VALIDATOR
    182232
    … …  
    193243
    194244    RTSEMRW sem;
     245
     246#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     247    com::Utf8Str strDescription;
     248#endif
    195249};
    196250
    … …  
    200254    int vrc = RTSemRWCreate(&m->sem);
    201255    AssertRC(vrc);
     256
     257#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     258    m->strDescription = com::Utf8StrFmt("r/w %RCv", this);
     259#endif
    202260}
    203261
    … …  
    255313}
    256314
     315#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     316/*virtual*/ const char* RWLockHandle::describe() const
     317{
     318    return m->strDescription.c_str();
     319}
     320#endif
     321
    257322////////////////////////////////////////////////////////////////////////////////
    258323//
    … …  
    267332
    268333    mutable RTCRITSECT sem;
     334
     335#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     336    com::Utf8Str strDescription;
     337#endif
    269338};
    270339
    … …  
    273342    m = new Data;
    274343    RTCritSectInit(&m->sem);
     344
     345#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     346    m->strDescription = com::Utf8StrFmt("crit %RCv", this);
     347#endif
    275348}
    276349
    … …  
    325398    return RTCritSectGetRecursion(&m->sem);
    326399}
     400
     401#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     402/*virtual*/ const char* WriteLockHandle::describe() const
     403{
     404    return m->strDescription.c_str();
     405}
     406#endif
    327407
    328408////////////////////////////////////////////////////////////////////////////////
    … …  
    501581}
    502582
     583#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     584void AutoLockBase::dumpStack(const char *pcszMessage, RT_SRC_POS_DECL)
     585{
     586    LockingStack *pStack = getThreadLocalLockingStack();
     587    LogFlow(("LOCKVAL DUMPSTACK at %s (%s:%u)\nLOCKVAL DUMPSTACK %s\n", pszFunction, pszFile, iLine, pcszMessage));
     588    dumpThreadLocalLockingStack(pStack);
     589}
     590#endif
     591
    503592////////////////////////////////////////////////////////////////////////////////
    504593//
    … …  
    611700
    612701    // unlock in reverse order!
    613     uint32_t i = 0;
     702    uint32_t i = m->aHandles.size();
    614703    for (HandlesVector::reverse_iterator it = m->aHandles.rbegin();
    615704         it != m->aHandles.rend();
    616705         ++it)
    617706    {
     707        --i;            // array index is zero based, decrement with every loop since we iterate backwards
    618708        LockHandle *pHandle = *it;
    619709        if (pHandle)
    … …  
    623713            AssertMsg(m->acUnlockedInLeave[i] >= 1, ("m->cUnlockedInLeave[%d] is %d, must be >=1!", i, m->acUnlockedInLeave[i]));
    624714
     715#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     716            LogFlowFunc(("LOCKVAL: will unlock handle %d [%s] %d times\n", i, pHandle->describe(), m->acUnlockedInLeave[i]));
     717#endif
     718
    625719            for (uint32_t left = m->acUnlockedInLeave[i];
    626720                 left;
    … …  
    628722                callUnlockImpl(*pHandle);
    629723        }
    630         ++i;
    631724    }
    632725}
    … …  
    652745            AssertMsg(m->acUnlockedInLeave[i] != 0, ("m->cUnlockedInLeave[%d] is 0! enter() without leave()?", i));
    653746
     747#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     748            LogFlowFunc(("LOCKVAL: will lock handle %d [%s] %d times\n", i, pHandle->describe(), m->acUnlockedInLeave[i]));
     749#endif
     750
    654751            for (; m->acUnlockedInLeave[i]; --m->acUnlockedInLeave[i])
    655752                callLockImpl(*pHandle);
    … …  
    667764{
    668765    // unlock in reverse order!
    669     uint32_t i = 0;
     766    uint32_t i = m->aHandles.size();
    670767    for (HandlesVector::reverse_iterator it = m->aHandles.rbegin();
    671768         it != m->aHandles.rend();
    672769         ++it)
    673770    {
     771        --i;            // array index is zero based, decrement with every loop since we iterate backwards
    674772        LockHandle *pHandle = *it;
    675773        if (pHandle)
    … …  
    680778                AssertMsg(m->acUnlockedInLeave[i] >= 1, ("m->cUnlockedInLeave[%d] is %d, must be >=1!", i, m->acUnlockedInLeave[i]));
    681779
     780#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     781                LogFlowFunc(("LOCKVAL: will unlock handle %d [%s] %d times\n", i, pHandle->describe(), m->acUnlockedInLeave[i]));
     782#endif
     783
    682784                for (uint32_t left = m->acUnlockedInLeave[i];
    683785                     left;
    … …  
    707809            if (!pHandle->isWriteLockOnCurrentThread())
    708810            {
     811#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     812                LogFlowFunc(("LOCKVAL: will lock handle %d [%s] %d times\n", i, pHandle->describe(), m->acUnlockedInLeave[i]));
     813#endif
     814
    709815                for (; m->acUnlockedInLeave[i]; --m->acUnlockedInLeave[i])
    710816                    callLockImpl(*pHandle);

  • trunk/src/VBox/Main/include/AutoLock.h

    r25310 r25408  
    11/** @file
    22 *
    3  * AutoWriteLock/AutoReadLock: smart R/W semaphore wrappers
     3 * Automatic locks, implementation
    44 */
    55
    … …  
    2525#include <iprt/types.h>
    2626
     27// macros for automatic lock validation;
     28// use VBOX_WITH_LOCK_VALIDATOR to enable for debug mode
    2729#ifdef DEBUG
    2830# ifdef VBOX_WITH_LOCK_VALIDATOR
    … …  
    5658/**
    5759 * Abstract base class for semaphore handles (RWLockHandle and WriteLockHandle).
     60 * Don't use this directly, but this implements lock validation for them.
    5861 */
    5962class LockHandle
    … …  
    7982    virtual uint32_t writeLockLevel() const = 0;
    8083
    81 #ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
    82     void validateLock(LOCKVAL_SRC_POS_DECL);
    83     void validateUnlock();
    84 #endif
    85 
    8684    virtual void lockWrite(LOCKVAL_SRC_POS_DECL) = 0;
    8785    virtual void unlockWrite() = 0;
    … …  
    8987    virtual void unlockRead() = 0;
    9088
     89#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     90    void validateLock(LOCKVAL_SRC_POS_DECL);
     91    void validateUnlock();
     92    virtual const char* describe() const = 0;
     93#endif
     94
    9195    static RTTLS s_lockingStackTlsIndex;
    9296
    … …  
    119123
    120124    virtual uint32_t writeLockLevel() const;
     125
     126#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     127    virtual const char* describe() const;
     128#endif
    121129
    122130private:
    … …  
    151159    virtual uint32_t writeLockLevel() const;
    152160
     161#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     162    virtual const char* describe() const;
     163#endif
     164
    153165private:
    154166    struct Data;
    … …  
    235247    void release();
    236248
     249#ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
     250    void dumpStack(const char *pcszMessage, RT_SRC_POS_DECL);
     251#endif
     252
    237253private:
    238254    // prohibit copy + assignment
    … …  
    250266 * Automatic read lock. Use this with a RWLockHandle to request a read/write
    251267 * semaphore in read mode. You can also use this with a WriteLockHandle but
    252  * that makes little sense since they know no read mode.
     268 * that makes little sense since they treat read mode like write mode.
    253269 *
    254270 * If constructed with a RWLockHandle or an instance of Lockable (which in
    … …  
    342358 *
    343359 * This cannot be used directly. Use AutoWriteLock or AutoMultiWriteLock2/3
    344  * which directly and indirectly derive from this.
     360 * which derive from this.
    345361 *
    346362 * In addition to utility methods for subclasses, this implements the public
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette