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 <mvanzin@vmware.com>

diff --git a/open-vm-tools/lib/lock/ul.c b/open-vm-tools/lib/lock/ul.c
index 01c1464c36221c9a9bfe1396aeb123610976de37..961cc9cf9a34a49fad46ae22029d5cdf177a8f41 100644 (file)
--- 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 f2b25cae0d685c5e833f7283b0ad826bba3168ba..6bdcc0d677de34c03d539ccf13ef488c725ae2f5 100644 (file)
--- 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;
 
 /*