lib/lock: finally put back the fix

This heavily reviewed change goes back in with one exception -
the part of the change that altered the size of structures isn't
included. The altering of structure sizes is causing hangs and
crashes. Hopefully the run-time checking, already in, will catch
the problems and we can clean those up. Once cleaned up we will
be free to alter structure sizes.

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 b906ddc297aaa7b1584045a6e1999fa61078dfb2..df26781f3507e8f358b7b27eeaa1c06119e37221 100644 (file)
--- a/open-vm-tools/lib/lock/ul.c
+++ b/open-vm-tools/lib/lock/ul.c
@@ -179,6 +179,10 @@ typedef struct {
 
 static Atomic_Ptr hashTableMem;
 
+#if defined(_WIN32) && !defined(VMX86_VMX)
+static Atomic_Ptr hashLockMem;
+#endif
+
 
 /*
  *-----------------------------------------------------------------------------
@@ -208,8 +212,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? */
@@ -235,6 +249,10 @@ MXUserGetPerThread(void *tid,      // IN: thread ID
       }
    }
 
+#if defined(_WIN32) && !defined(VMX86_VMX)
+   MXRecLockRelease(hashLock);
+#endif
+
    return perThread;
 }
 
@@ -516,6 +534,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
 }
 
 

diff --git a/open-vm-tools/lib/lock/ulInt.h b/open-vm-tools/lib/lock/ulInt.h
index a3da3b9664bdeac0e025ff62690b8d0fd70c0c28..64c8ffc8e19ca0b73a3e2bb7edf615cc60de6d22 100644 (file)
--- a/open-vm-tools/lib/lock/ulInt.h
+++ b/open-vm-tools/lib/lock/ulInt.h
@@ -388,7 +388,22 @@ MXRecLockRelease(MXRecLock *lock)  // IN/OUT:
 static INLINE void *
 MXUserGetThreadID(void)
 {
+#if defined(_WIN32)
+   /*
+    * On Windows there is a problem with using VThread_CurID() - it doesn't
+    * maintain unique thread ID values (PR 780775). Native thread ID values
+    * and special handling are used to resolve issues.
+    */
+
+   return (void *) (uintptr_t) GetCurrentThreadId();  // DWORD
+#else
+   /*
+    * Outside of Windows there are no known issues with using VThread_CurID
+    * so that is what is used.
+    */
+
    return (void *) (uintptr_t) VThread_CurID();  // unsigned
+#endif
 }
 
 /*