From: VMware, Inc <> Date: Mon, 21 Nov 2011 23:51:00 +0000 (-0800) Subject: lib/lock: things go back X-Git-Tag: 2011.11.20-535097~21 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b0e9701c9e05b417cc53134ffa371b4b842012d0;p=thirdparty%2Fopen-vm-tools.git lib/lock: things go back The entire new lib/lock structure is back in... except that there is an ifdef for working the new way. By default it will run compatible with the old way. I've discovered that there are as many as 5 copies of lib/lock in some of our products (i.e. Linux UI). Things would be fine if the copies were used in a self contained fashion. When lib/lock runs in the new way, random syndrome bits per library caused crashes and hangs proving we're leaking locks across a boundary (something unexpected). Two copies of lib/lock passing around locks could be fatal if the two copies are different versions of lib/lock. Unfortunately logs and core dumps haven't been easy to find. I'm going to begin a detailed search for the root cause via the ifdef; others should not be affected. Signed-off-by: Marcelo Vanzin --- diff --git a/open-vm-tools/lib/lock/ul.c b/open-vm-tools/lib/lock/ul.c index 01c1464c3..961cc9cf9 100644 --- a/open-vm-tools/lib/lock/ul.c +++ b/open-vm-tools/lib/lock/ul.c @@ -23,6 +23,7 @@ #include "ulInt.h" #include "ulIntShared.h" #include "hashTable.h" +#include "random.h" static Bool mxInPanic = FALSE; // track when involved in a panic @@ -84,6 +85,61 @@ MXUserInternalSingleton(Atomic_Ptr *storage) // IN: return lock; } +#if defined(MXUSER_SYNDROME) +/* + *----------------------------------------------------------------------------- + * + * MXUserSydrome -- + * + * Generate the syndrome bits for this MXUser library. + * + * Each MXUser library has unique syndrome bits enabling the run time + * detection of locks created with one copy of the MXUser library and + * passed to another copy of the MXUser library. + * + * The syndrome bits are important as they prevent incompatible versions + * of the MXUser library from trashing each other. + * + * The bits are generated by using a source of bits that is external to + * a program and its libraries. This way no code or data based scheme + * can be spoofed or aliased. + * + * Results: + * As above + * + * Side effects: + * None + * + *----------------------------------------------------------------------------- + */ + +static uint32 +MXUserSyndrome(void) +{ + uint32 syndrome; + static Atomic_uint32 syndromeMem; // implicitly zero -- mbellon + + syndrome = Atomic_Read(&syndromeMem); + + if (syndrome == 0) { + while (Random_Crypto(sizeof syndrome, &syndrome) && (syndrome == 0)) + ; + + if (syndrome == 0) { + syndrome++; // Fudge in case of failure + } + + /* blind write; if racing one thread or the other will do */ + Atomic_ReadIfEqualWrite(&syndromeMem, 0, syndrome); + + syndrome = Atomic_Read(&syndromeMem); + ASSERT(syndrome); + } + + return syndrome; +} +#endif + /* *----------------------------------------------------------------------------- @@ -92,9 +148,6 @@ MXUserInternalSingleton(Atomic_Ptr *storage) // IN: * * Return a signature appropriate for the specified object type. * - * TODO: this will combine random syndrome bits (making each lib/lock - * unique) and bits representing the objectType. - * * Results: * As above * @@ -107,9 +160,64 @@ MXUserInternalSingleton(Atomic_Ptr *storage) // IN: uint32 MXUserGetSignature(MXUserObjectType objectType) // IN: { - return objectType; // Unity mapping, for now -} + uint32 signature; + ASSERT((objectType != MXUSER_TYPE_NEVER_USE) && (objectType < 16)); + +#if defined(MXUSER_SYNDROME) + /* + * Use a random syndrome combined with a unique bit pattern mapping + * of objectType to bits in a nibble. The random portion of the signature + * can be used to catch multiple copies of lib/lock that are "leaking" + * locks between them (which may be incompatible due to internal changes). + */ + + signature = (MXUserSyndrome() & 0x0FFFFFFF) | (objectType << 28); +#else + /* + * Map the abstract objectType back to the bit patterns used in older + * versions of lib/lock. This provides absolute compatibility with + * these older libraries. + */ + + switch (objectType) { + case MXUSER_TYPE_RW: + signature = 0x57524B4C; + break; + + case MXUSER_TYPE_REC: + signature = 0x43524B4C; + break; + + case MXUSER_TYPE_RANK: + signature = 0x4E4B5241; + break; + + case MXUSER_TYPE_EXCL: + signature = 0x58454B4C; + break; + + case MXUSER_TYPE_SEMA: + signature = 0x414D4553; + break; + + case MXUSER_TYPE_CONDVAR: + signature = 0x444E4F43; + break; + + case MXUSER_TYPE_BARRIER: + signature = 0x52524142; + break; + + default: + Panic("%s: unknown objectType %d\n", __FUNCTION__, objectType); + } +#endif + + ASSERT(signature); + + return signature; +} #if defined(MXUSER_DEBUG) #define MXUSER_MAX_LOCKS_PER_THREAD (2 * MXUSER_MAX_REC_DEPTH) @@ -121,6 +229,10 @@ typedef struct { static Atomic_Ptr hashTableMem; +#if defined(_WIN32) && !defined(VMX86_VMX) +static Atomic_Ptr hashLockMem; +#endif + /* *----------------------------------------------------------------------------- @@ -150,8 +262,18 @@ MXUserGetPerThread(void *tid, // IN: thread ID HashTable *hash; MXUserPerThread *perThread = NULL; +#if defined(_WIN32) && !defined(VMX86_VMX) + MXRecLock *hashLock = MXUserInternalSingleton(&hashLockMem); + + ASSERT(hashLock); + + hash = HashTable_AllocOnce(&hashTableMem, 1024, HASH_INT_KEY, NULL); + + MXRecLockAcquire(hashLock); +#else hash = HashTable_AllocOnce(&hashTableMem, 1024, HASH_INT_KEY | HASH_FLAG_ATOMIC, NULL); +#endif if (!HashTable_Lookup(hash, tid, (void **) &perThread)) { /* No entry for this tid was found, allocate one? */ @@ -177,6 +299,10 @@ MXUserGetPerThread(void *tid, // IN: thread ID } } +#if defined(_WIN32) && !defined(VMX86_VMX) + MXRecLockRelease(hashLock); +#endif + return perThread; } @@ -458,6 +584,34 @@ MXUserReleaseTracking(MXUserHeader *header) // IN: lock, via its header perThread->lockArray[lastEntry] = NULL; // tidy up memory perThread->locksHeld--; + +#if defined(_WIN32) && !defined(VMX86_VMX) + /* + * On Windows thread IDs aren't greedily recycled. If a process creates and + * destroys many threads this can cause a memory leak of perThread data + * (and its overhead). We avoid this by atomically (via a lock) creating + * (upon first lock acquired) and deleting (upon last lock release) a + * perThread. + * + * Yes, this is a performance cost but it only affects Windows debug + * builds and then not by very much - we tend to run for a long time + * with either no locks held or at least one lock held. + */ + + if (perThread->locksHeld == 0) { + HashTable *hash = Atomic_ReadPtr(&hashTableMem); + MXRecLock *hashLock = MXUserInternalSingleton(&hashLockMem); + + ASSERT(hash); + ASSERT(hashLock); + + MXRecLockAcquire(hashLock); + HashTable_Delete(hash, tid); + MXRecLockRelease(hashLock); + + free(perThread); + } +#endif } @@ -516,9 +670,9 @@ MXUserValidateHeader(MXUserHeader *header, // IN: __FUNCTION__, expected, header->signature); } -// if (header->serialNumber == 0) { -// MXUserDumpAndPanic(header, "%s: Invalid serial number!", __FUNCTION__); -// } + if (header->serialNumber == 0) { + MXUserDumpAndPanic(header, "%s: Invalid serial number!", __FUNCTION__); + } } #endif diff --git a/open-vm-tools/lib/lock/ulInt.h b/open-vm-tools/lib/lock/ulInt.h index f2b25cae0..6bdcc0d67 100644 --- a/open-vm-tools/lib/lock/ulInt.h +++ b/open-vm-tools/lib/lock/ulInt.h @@ -396,13 +396,14 @@ MXUserGetThreadID(void) */ typedef enum { - MXUSER_TYPE_RW = 0x57524B4C, // *MUST* *NEVER* start at zero - MXUSER_TYPE_REC = 0x43524B4C, - MXUSER_TYPE_RANK = 0x4E4B5241, - MXUSER_TYPE_EXCL = 0x58454B4C, - MXUSER_TYPE_SEMA = 0x414D4553, - MXUSER_TYPE_CONDVAR = 0x444E4F43, - MXUSER_TYPE_BARRIER = 0x52524142 + MXUSER_TYPE_NEVER_USE = 0, + MXUSER_TYPE_RW = 1, + MXUSER_TYPE_REC = 2, + MXUSER_TYPE_RANK = 3, + MXUSER_TYPE_EXCL = 4, + MXUSER_TYPE_SEMA = 5, + MXUSER_TYPE_CONDVAR = 6, + MXUSER_TYPE_BARRIER = 7 } MXUserObjectType; /*