xplat: Improve spinLock and criticalSection implementations

Measured on amd64, new CCSpinLock works twice faster comparing to older
implementation and uses a lighter interface. Besides, previous
interface was interfering with blocking threads.

lib/Common/Core/CriticalSection.h

@@ -5,6 +5,13 @@
 #pragma once
 
 class CriticalSection
+#ifndef _WIN32
+: public CCSpinLock<true>
+{
+public:
+    CriticalSection(DWORD spincount = 0): CCSpinLock(spincount) { }
+};
+#else // _WIN32
 {
 public:
     CriticalSection(DWORD spincount = 0)
@@ -22,6 +29,7 @@ public:
 private:
     CRITICAL_SECTION cs;
 };
+#endif
 
 //FakeCriticalSection mimics CriticalSection apis
 class FakeCriticalSection
@@ -102,4 +110,3 @@ public:
 private:
     SyncObject * cs;
 };
-

lib/Common/DataStructures/DictionaryStats.cpp

@@ -7,7 +7,7 @@
 #include "DictionaryStats.h"
 
 DictionaryType* DictionaryStats::dictionaryTypes = NULL;
-CRITICAL_SECTION DictionaryStats::dictionaryTypesCriticalSection;
+CriticalSection DictionaryStats::dictionaryTypesCriticalSection;
 
 DictionaryStats* DictionaryStats::Create(const char* name, uint bucketCount)
 {
@@ -49,11 +49,7 @@ DictionaryStats::DictionaryStats(const char* name, uint bucketCount)
     pNext(NULL),
     pName(NULL)
 {
-    if(dictionaryTypes == NULL)
-    {
-        InitializeCriticalSection(&DictionaryStats::dictionaryTypesCriticalSection);
-    }
-    EnterCriticalSection(&DictionaryStats::dictionaryTypesCriticalSection);
+    DictionaryStats::dictionaryTypesCriticalSection.Enter();
     DictionaryType* type = NULL;
     // See if we already created instance(s) of this type
     DictionaryType* current = dictionaryTypes;
@@ -77,7 +73,7 @@ DictionaryStats::DictionaryStats(const char* name, uint bucketCount)
         strncpy_s(type->name, name, _countof(type->name)-1);
         type->name[sizeof(type->name)-1]='\0';
     }
-    LeaveCriticalSection(&dictionaryTypesCriticalSection);
+    dictionaryTypesCriticalSection.Leave();
     // keep a pointer to the name in case we are asked to clone ourselves
     pName = type->name;
 
@@ -128,7 +124,7 @@ void DictionaryStats::OutputStats()
     if (!dictionaryTypes)
         return;
 
-    EnterCriticalSection(&DictionaryStats::dictionaryTypesCriticalSection);
+    DictionaryStats::dictionaryTypesCriticalSection.Enter();
     DictionaryType* current = dictionaryTypes;
     Output::Print(_u("PROFILE DICTIONARY\n"));
     Output::Print(_u("%8s  %13s  %13s  %13s  %13s  %13s  %13s  %13s  %14s  %14s  %13s  %13s  %13s    %s\n"), _u("Metric"),_u("StartSize"), _u("EndSize"), _u("Resizes"), _u("Items"), _u("MaxDepth"), _u("EmptyBuckets"), _u("Lookups"), _u("Collisions"), _u("AvgLookupDepth"), _u("AvgCollDepth"), _u("MaxLookupDepth"), _u("Instances"), _u("Type"));
@@ -208,8 +204,7 @@ void DictionaryStats::OutputStats()
     }
     Output::Print(_u("====================================================================================\n"));
     ClearStats();
-    LeaveCriticalSection(&DictionaryStats::dictionaryTypesCriticalSection);
-    DeleteCriticalSection(&DictionaryStats::dictionaryTypesCriticalSection);
+    DictionaryStats::dictionaryTypesCriticalSection.Leave();
 }
 
 void DictionaryStats::ComputeStats(uint input, double &total, double &max)

lib/Common/DictionaryStats.h

@@ -27,7 +27,7 @@ private:
 
     static DictionaryType* dictionaryTypes;
 
-    static CRITICAL_SECTION dictionaryTypesCriticalSection;
+    static CriticalSection dictionaryTypesCriticalSection;
 
 public:
     void Resize(uint newSize, uint emptyBucketCount);

lib/Common/Memory/ArenaAllocator.h

@@ -690,9 +690,9 @@ public:
 #endif
     {}
 
-    void SetHasUsedInlineCache(bool value) 
-    { 
-        hasUsedInlineCache = value; 
+    void SetHasUsedInlineCache(bool value)
+    {
+        hasUsedInlineCache = value;
 #if DBG
         if (hasUsedInlineCache)
         {
@@ -806,7 +806,7 @@ class CacheAllocator : public ArenaAllocatorBase<CacheAllocatorTraits>
 {
 public:
     CacheAllocator(__in LPCWSTR name, PageAllocator * pageAllocator, void(*outOfMemoryFunc)()) :
-        ArenaAllocatorBase<CacheAllocatorTraits>(name, pageAllocator, outOfMemoryFunc) 
+        ArenaAllocatorBase<CacheAllocatorTraits>(name, pageAllocator, outOfMemoryFunc)
 #if DBG
         , verifiedAllZeroAndLockedDown(false)
 #endif
@@ -932,7 +932,7 @@ public:
 // Strong references should be short lived
 class ReferencedArenaAdapter : public RefCounted
 {
-    CRITICAL_SECTION adapterLock;
+    CriticalSection adapterLock;
     uint32 strongRefCount;
     ArenaAllocator* arena;
     bool deleteFlag;
@@ -944,7 +944,6 @@ public:
         {
             HeapDelete(this->arena);
         }
-        DeleteCriticalSection(&adapterLock);
     }
 
     ReferencedArenaAdapter(ArenaAllocator* _arena)
@@ -952,13 +951,12 @@ public:
           strongRefCount(0),
           arena(_arena),
           deleteFlag(false)
-    {
-        InitializeCriticalSection(&adapterLock);
-    }
+    { }
 
     bool AddStrongReference()
     {
-        EnterCriticalSection(&adapterLock);
+        bool retval = false;
+        adapterLock.Enter();
 
         if (deleteFlag)
         {
@@ -969,21 +967,22 @@ public:
                 HeapDelete(this->arena);
                 this->arena = nullptr;
             }
-            LeaveCriticalSection(&adapterLock);
-            return false;
         }
         else
         {
             // Succeed at acquiring a Strong Reference into the Arena
             strongRefCount++;
-            LeaveCriticalSection(&adapterLock);
-            return true;
+            retval = true;
         }
+
+        adapterLock.Leave();
+
+        return retval;
     }
 
     void ReleaseStrongReference()
     {
-        EnterCriticalSection(&adapterLock);
+        adapterLock.Enter();
         strongRefCount--;
 
         if (deleteFlag && this->arena && 0 == strongRefCount)
@@ -993,13 +992,13 @@ public:
             this->arena = NULL;
         }
 
-        LeaveCriticalSection(&adapterLock);
+        adapterLock.Leave();
     }
 
     void DeleteArena()
     {
         deleteFlag = true;
-        if (TryEnterCriticalSection(&adapterLock))
+        if (adapterLock.TryEnter())
         {
             if (0 == strongRefCount)
             {
@@ -1007,7 +1006,7 @@ public:
                 HeapDelete(this->arena);
                 this->arena = NULL;
             }
-            LeaveCriticalSection(&adapterLock);
+            adapterLock.Leave();
         }
     }
 

lib/Common/Memory/Recycler.cpp

@@ -255,6 +255,9 @@ Recycler::Recycler(AllocationPolicyManager * policyManager, IdleDecommitPageAllo
 #if GLOBAL_ENABLE_WRITE_BARRIER
     , pendingWriteBarrierBlockMap(&HeapAllocator::Instance)
 #endif
+#ifdef PROFILE_RECYCLER_ALLOC
+    , trackerCriticalSection(nullptr)
+#endif
 {
 #ifdef RECYCLER_MARK_TRACK
     this->markMap = NoCheckHeapNew(MarkMap, &NoCheckHeapAllocator::Instance, 163, &markMapCriticalSection);
@@ -574,7 +577,7 @@ Recycler::~Recycler()
         });
         NoCheckHeapDelete(this->trackerDictionary);
         this->trackerDictionary = nullptr;
-        ::DeleteCriticalSection(&trackerCriticalSection);
+        delete(trackerCriticalSection);
     }
 #endif
 
@@ -7249,7 +7252,7 @@ void Recycler::VerifyPageHeapFillAfterAlloc(char* memBlock, size_t size, ObjectI
         if (heapBlock->IsLargeHeapBlock())
         {
             LargeHeapBlock* largeHeapBlock = (LargeHeapBlock*)heapBlock;
-            if (largeHeapBlock->InPageHeapMode() 
+            if (largeHeapBlock->InPageHeapMode()
 #ifdef RECYCLER_NO_PAGE_REUSE
                 && !largeHeapBlock->GetPageAllocator(this)->IsPageReuseDisabled()
 #endif
@@ -7614,9 +7617,8 @@ Recycler::InitializeProfileAllocTracker()
     if (DoProfileAllocTracker())
     {
         trackerDictionary = NoCheckHeapNew(TypeInfotoTrackerItemMap, &NoCheckHeapAllocator::Instance, 163);
-
+        trackerCriticalSection = new CriticalSection(1000);
 #pragma prefast(suppress:6031, "InitializeCriticalSectionAndSpinCount always succeed since Vista. No need to check return value");
-        InitializeCriticalSectionAndSpinCount(&trackerCriticalSection, 1000);
     }
 
     nextAllocData.Clear();
@@ -7699,9 +7701,9 @@ void* Recycler::TrackAlloc(void* object, size_t size, const TrackAllocData& trac
     if (this->trackerDictionary != nullptr)
     {
         Assert(nextAllocData.IsEmpty()); // should have been cleared
-        EnterCriticalSection(&trackerCriticalSection);
+        trackerCriticalSection->Enter();
         TrackAllocCore(object, size, trackAllocData);
-        LeaveCriticalSection(&trackerCriticalSection);
+        trackerCriticalSection->Leave();
     }
     return object;
 }
@@ -7712,7 +7714,7 @@ Recycler::TrackIntegrate(__in_ecount(blockSize) char * blockAddress, size_t bloc
     if (this->trackerDictionary != nullptr)
     {
         Assert(nextAllocData.IsEmpty()); // should have been cleared
-        EnterCriticalSection(&trackerCriticalSection);
+        trackerCriticalSection->Enter();
 
         char * address = blockAddress;
         char * blockEnd = blockAddress + blockSize;
@@ -7722,7 +7724,7 @@ Recycler::TrackIntegrate(__in_ecount(blockSize) char * blockAddress, size_t bloc
             address += allocSize;
         }
 
-        LeaveCriticalSection(&trackerCriticalSection);
+        trackerCriticalSection->Leave();
     }
 }
 
@@ -7730,7 +7732,7 @@ BOOL Recycler::TrackFree(const char* address, size_t size)
 {
     if (this->trackerDictionary != nullptr)
     {
-        EnterCriticalSection(&trackerCriticalSection);
+        trackerCriticalSection->Enter();
         TrackerData * data = GetTrackerData((char *)address);
         if (data != nullptr)
         {
@@ -7762,7 +7764,7 @@ BOOL Recycler::TrackFree(const char* address, size_t size)
                 Assert(false);
             }
         }
-        LeaveCriticalSection(&trackerCriticalSection);
+        trackerCriticalSection->Leave();
     }
     return true;
 }
@@ -7791,14 +7793,14 @@ Recycler::TrackUnallocated(__in char* address, __in  char *endAddress, size_t si
     {
         if (this->trackerDictionary != nullptr)
         {
-            EnterCriticalSection(&trackerCriticalSection);
+            trackerCriticalSection->Enter();
             while (address + sizeCat <= endAddress)
             {
                 Assert(GetTrackerData(address) == nullptr);
                 SetTrackerData(address, &TrackerData::EmptyData);
                 address += sizeCat;
             }
-            LeaveCriticalSection(&trackerCriticalSection);
+            trackerCriticalSection->Leave();
         }
     }
 }
@@ -8035,7 +8037,7 @@ Recycler::VerifyMarkStack()
     }
 }
 
-bool 
+bool
 Recycler::VerifyMark(void * target)
 {
     return VerifyMark(nullptr, target);

lib/Common/Memory/Recycler.h

@@ -151,20 +151,20 @@ struct InfoBitsWrapper{};
 #endif
 
 #ifndef RECYCLER_WRITE_BARRIER
-#define RecyclerNewWithBarrier                          RecyclerNew                     
-#define RecyclerNewWithBarrierPlus                      RecyclerNewPlus                 
-#define RecyclerNewWithBarrierPlusZ                     RecyclerNewPlusZ                
-#define RecyclerNewWithBarrierZ                         RecyclerNewZ                    
-#define RecyclerNewWithBarrierStruct                    RecyclerNewStruct               
-#define RecyclerNewWithBarrierStructZ                   RecyclerNewStructZ              
-#define RecyclerNewWithBarrierStructPlus                RecyclerNewStructPlus           
-#define RecyclerNewWithBarrierArray                     RecyclerNewArray                
-#define RecyclerNewWithBarrierArrayZ                    RecyclerNewArrayZ               
-#define RecyclerNewWithBarrierFinalized                 RecyclerNewFinalized            
-#define RecyclerNewWithBarrierFinalizedPlus             RecyclerNewFinalizedPlus        
-#define RecyclerNewWithBarrierTracked                   RecyclerNewTracked              
-#define RecyclerNewWithBarrierEnumClass                 RecyclerNewEnumClass            
-#define RecyclerNewWithBarrierWithInfoBits              RecyclerNewWithInfoBits         
+#define RecyclerNewWithBarrier                          RecyclerNew
+#define RecyclerNewWithBarrierPlus                      RecyclerNewPlus
+#define RecyclerNewWithBarrierPlusZ                     RecyclerNewPlusZ
+#define RecyclerNewWithBarrierZ                         RecyclerNewZ
+#define RecyclerNewWithBarrierStruct                    RecyclerNewStruct
+#define RecyclerNewWithBarrierStructZ                   RecyclerNewStructZ
+#define RecyclerNewWithBarrierStructPlus                RecyclerNewStructPlus
+#define RecyclerNewWithBarrierArray                     RecyclerNewArray
+#define RecyclerNewWithBarrierArrayZ                    RecyclerNewArrayZ
+#define RecyclerNewWithBarrierFinalized                 RecyclerNewFinalized
+#define RecyclerNewWithBarrierFinalizedPlus             RecyclerNewFinalizedPlus
+#define RecyclerNewWithBarrierTracked                   RecyclerNewTracked
+#define RecyclerNewWithBarrierEnumClass                 RecyclerNewEnumClass
+#define RecyclerNewWithBarrierWithInfoBits              RecyclerNewWithInfoBits
 #define RecyclerNewWithBarrierFinalizedClientTracked    RecyclerNewFinalizedClientTracked
 #endif
 
@@ -302,7 +302,7 @@ enum CollectionFlags
     FinishConcurrentDefault         = CollectMode_Concurrent | CollectOverride_DisableIdleFinish | CollectOverride_BackgroundFinishMark,
     FinishConcurrentOnExitScript    = FinishConcurrentDefault,
     FinishConcurrentOnEnterScript   = FinishConcurrentDefault,
-    FinishConcurrentOnAllocation    = FinishConcurrentDefault, 
+    FinishConcurrentOnAllocation    = FinishConcurrentDefault,
     FinishDispose                   = CollectOverride_AllowDispose,
     FinishDisposeTimed              = CollectOverride_AllowDispose | CollectHeuristic_TimeIfScriptActive,
     ForceFinishCollection           = CollectOverride_ForceFinish | CollectOverride_ForceInThread,
@@ -693,7 +693,7 @@ private:
 public:
     template<typename Action>
     void ForEachPageAllocator(Action action)
-    {        
+    {
         action(&this->recyclerPageAllocator);
         action(&this->recyclerLargeBlockPageAllocator);
 #ifdef RECYCLER_WRITE_BARRIER_ALLOC_SEPARATE_PAGE
@@ -1264,10 +1264,10 @@ public:
     DEFINE_RECYCLER_ALLOC(AllocTrackedWithBarrier, ClientTrackableObjectWithBarrierBits);
     DEFINE_RECYCLER_ALLOC(AllocFinalizedClientTrackedWithBarrier, ClientFinalizableObjectWithBarrierBits);
 #endif
-    
+
     DEFINE_RECYCLER_ALLOC(AllocLeaf, LeafBit);
     DEFINE_RECYCLER_ALLOC(AllocFinalizedLeaf, FinalizableLeafBits);
-    DEFINE_RECYCLER_ALLOC(AllocTrackedLeaf, ClientTrackableLeafBits);    
+    DEFINE_RECYCLER_ALLOC(AllocTrackedLeaf, ClientTrackableLeafBits);
     DEFINE_RECYCLER_ALLOC_ZERO(AllocLeafZero, LeafBit);
     DEFINE_RECYCLER_ALLOC_ZERO(AllocZeroTrackedLeaf, ClientTrackableLeafBits);
     DEFINE_RECYCLER_NOTHROW_ALLOC_ZERO(AllocImplicitRootLeaf, ImplicitRootLeafBits);
@@ -1828,7 +1828,7 @@ private:
     typedef JsUtil::BaseDictionary<void *, TrackerData *, NoCheckHeapAllocator, PrimeSizePolicy, RecyclerPointerComparer, JsUtil::SimpleDictionaryEntry, JsUtil::NoResizeLock> PointerToTrackerDataMap;
 
     TypeInfotoTrackerItemMap * trackerDictionary;
-    CRITICAL_SECTION trackerCriticalSection;
+    CriticalSection * trackerCriticalSection;
 #endif
     TrackAllocData nextAllocData;
 #endif
@@ -2384,7 +2384,7 @@ public:
 
 // Partial template specialization to allocate as non leaf
 template <typename T>
-class TypeAllocatorFunc<RecyclerNonLeafAllocator, T> : 
+class TypeAllocatorFunc<RecyclerNonLeafAllocator, T> :
 #if GLOBAL_ENABLE_WRITE_BARRIER
     public _RecyclerAllocatorFunc<_RecyclerWriteBarrierPolicy>
 #else
@@ -2508,7 +2508,7 @@ operator new(DECLSPEC_GUARD_OVERFLOW size_t byteSize, Recycler * recycler, const
     AssertCanHandleOutOfMemory();
     Assert(byteSize != 0);
     void * buffer;
-    
+
     if (infoBits & EnumClass_1_Bit)
     {
         buffer = recycler->AllocEnumClass<infoBits>(byteSize);

pal/inc/CCSpinLock.hpp

@@ -0,0 +1,39 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+#ifndef CC_PAL_INC_CCSPINLOCK_H
+#define CC_PAL_INC_CCSPINLOCK_H
+
+template<bool shouldTrackThreadId>
+class CCSpinLock
+{
+    unsigned int  enterCount;
+    unsigned int  spinCount;
+    char          lock;
+    DWORD         threadId;
+public:
+    // TODO: (obastemur) tune this / don't use constant here?
+    void Reset(unsigned int spinCount = 11)
+    {
+        this->enterCount = 0;
+        this->lock = 0;
+        this->threadId = 0;
+        this->spinCount = spinCount;
+    }
+
+    CCSpinLock(unsigned int spinCount)
+    {
+        Reset(spinCount);
+    }
+
+    CCSpinLock() { Reset(); }
+
+    void Enter();
+    bool TryEnter();
+    void Leave();
+    bool IsLocked() const;
+};
+
+#endif // CC_PAL_INC_CCSPINLOCK_H

pal/inc/pal.h

@@ -6878,6 +6878,8 @@ ULONG_PTR __stdcall GetCurrentSP();
 
 #ifdef  __cplusplus
 }
+
+#include "CCSpinLock.hpp"
 #endif
 
 #endif // __PAL_H__

pal/src/CMakeLists.txt

@@ -162,6 +162,7 @@ set(SOURCES
   synchmgr/synchmanager.cpp
   synchmgr/wait.cpp
   sync/cs.cpp
+  sync/CCSpinLock.cpp
   thread/context.cpp
   thread/process.cpp
   thread/pal_thread.cpp

pal/src/config.h.in

@@ -131,7 +131,6 @@
 #define MKSTEMP64_IS_USED_INSTEAD_OF_MKSTEMP 0
 #define NEED_DLCOMPAT 0
 #define OPEN64_IS_USED_INSTEAD_OF_OPEN 0
-#define PAL_IGNORE_NORMAL_THREAD_PRIORITY 0
 #define SELF_SUSPEND_FAILS_WITH_NATIVE_SUSPENSION 0
 #define SET_SCHEDPARAM_NEEDS_PRIVS 0
 #define SIGWAIT_FAILS_WHEN_PASSED_FULL_SIGSET 0

pal/src/configure.cmake

@@ -1011,7 +1011,6 @@ else() # ANDROID
   set(MKSTEMP64_IS_USED_INSTEAD_OF_MKSTEMP 0)
   set(NEED_DLCOMPAT 0)
   # set(OPEN64_IS_USED_INSTEAD_OF_OPEN 0)
-  set(PAL_IGNORE_NORMAL_THREAD_PRIORITY 0)
   set(SELF_SUSPEND_FAILS_WITH_NATIVE_SUSPENSION 0)
   set(SET_SCHEDPARAM_NEEDS_PRIVS 0)
   set(SIGWAIT_FAILS_WHEN_PASSED_FULL_SIGSET 0)

pal/src/include/pal/corunix.hpp

@@ -1,6 +1,6 @@
 //
 // Copyright (c) Microsoft. All rights reserved.
-// Licensed under the MIT license. See LICENSE file in the project root for full license information. 
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
 //
 
 /*++
@@ -23,6 +23,7 @@ Abstract:
 #define _CORUNIX_H
 
 #include "palinternal.h"
+#include "CCSpinLock.hpp"
 
 namespace CorUnix
 {
@@ -55,8 +56,8 @@ namespace CorUnix
         //
         // Length of string, not including terminating NULL
         //
-        
-        DWORD m_dwStringLength; 
+
+        DWORD m_dwStringLength;
 
         //
         // Length of buffer backing string; must be at least 1+dwStringLength
@@ -73,7 +74,7 @@ namespace CorUnix
             m_dwMaxLength(0)
         {
         };
-            
+
         CPalString(
             const WCHAR *pwsz
             )
@@ -126,7 +127,7 @@ namespace CorUnix
         {
             return m_dwMaxLength;
         };
-        
+
     };
 
     //
@@ -212,7 +213,7 @@ namespace CorUnix
     // persist security information for the object type (as would be the case
     // for, say, files) eSecurityPersistence should be set to
     // OSPersistedSecurityInfo.
-    // 
+    //
     // If the object may have a name eObjectNameSupport should be
     // ObjectCanHaveName. A named object can be opened in more than one
     // process.
@@ -301,14 +302,14 @@ namespace CorUnix
             NoOwner,
             OwnershipNotApplicable
         };
-        
+
     private:
 
         //
         // Array that maps object type IDs to the corresponding
         // CObjectType instance
         //
-        
+
         static CObjectType* s_rgotIdMapping[];
 
         PalObjectTypeId m_eTypeId;
@@ -383,7 +384,7 @@ namespace CorUnix
         {
             return m_eTypeId;
         };
-        
+
         OBJECTCLEANUPROUTINE
         GetObjectCleanupRoutine(
             void
@@ -391,7 +392,7 @@ namespace CorUnix
         {
             return m_pCleanupRoutine;
         };
-        
+
         OBJECTINITROUTINE
         GetObjectInitRoutine(
             void
@@ -399,7 +400,7 @@ namespace CorUnix
         {
             return  m_pInitRoutine;
         };
-        
+
         DWORD
         GetImmutableDataSize(
             void
@@ -407,7 +408,7 @@ namespace CorUnix
         {
             return  m_dwImmutableDataSize;
         };
-        
+
         DWORD
         GetProcessLocalDataSize(
             void
@@ -415,7 +416,7 @@ namespace CorUnix
         {
             return m_dwProcessLocalDataSize;
         };
-        
+
         DWORD
         GetSharedDataSize(
             void
@@ -423,7 +424,7 @@ namespace CorUnix
         {
             return m_dwSharedDataSize;
         };
-        
+
         DWORD
         GetSupportedAccessRights(
             void
@@ -431,7 +432,7 @@ namespace CorUnix
         {
             return m_dwSupportedAccessRights;
         };
-        
+
         // Generic access rights mapping
 
         SecuritySupport
@@ -441,7 +442,7 @@ namespace CorUnix
         {
             return  m_eSecuritySupport;
         };
-        
+
         SecurityPersistence
         GetSecurityPersistence(
             void
@@ -449,7 +450,7 @@ namespace CorUnix
         {
             return  m_eSecurityPersistence;
         };
-        
+
         ObjectNameSupport
         GetObjectNameSupport(
             void
@@ -457,7 +458,7 @@ namespace CorUnix
         {
             return  m_eObjectNameSupport;
         };
-        
+
         HandleDuplicationSupport
         GetHandleDuplicationSupport(
             void
@@ -465,7 +466,7 @@ namespace CorUnix
         {
             return  m_eHandleDuplicationSupport;
         };
-        
+
         SynchronizationSupport
         GetSynchronizationSupport(
             void
@@ -473,7 +474,7 @@ namespace CorUnix
         {
             return  m_eSynchronizationSupport;
         };
-        
+
         SignalingSemantics
         GetSignalingSemantics(
             void
@@ -481,7 +482,7 @@ namespace CorUnix
         {
             return  m_eSignalingSemantics;
         };
-        
+
         ThreadReleaseSemantics
         GetThreadReleaseSemantics(
             void
@@ -489,7 +490,7 @@ namespace CorUnix
         {
             return  m_eThreadReleaseSemantics;
         };
-        
+
         OwnershipSemantics
         GetOwnershipSemantics(
             void
@@ -556,7 +557,7 @@ namespace CorUnix
     class CObjectAttributes
     {
     public:
-        
+
         CPalString sObjectName;
         LPSECURITY_ATTRIBUTES pSecurityAttributes;
 
@@ -692,7 +693,7 @@ namespace CorUnix
         // not possible for the wait to be immediately satisfied.
         //
 
-        virtual        
+        virtual
         PAL_ERROR
         CanThreadWaitWithoutBlocking(
             bool *pfCanWaitWithoutBlocking,     // OUT
@@ -759,7 +760,7 @@ namespace CorUnix
 
     //
     // The following two enums are part of the local object
-    // optimizations 
+    // optimizations
     //
 
     enum ObjectDomain
@@ -894,7 +895,7 @@ namespace CorUnix
         GetObjectSynchData(
             VOID **ppvSynchData             // OUT
             ) = 0;
-        
+
     };
 
     class IPalProcess
@@ -906,7 +907,7 @@ namespace CorUnix
             void
             ) = 0;
     };
-    
+
     class IPalObjectManager
     {
     public:
@@ -969,7 +970,7 @@ namespace CorUnix
         // is needed for the OpenXXX routines and DuplicateHandle.
         //
 
-        virtual            
+        virtual
         PAL_ERROR
         LocateObject(
             CPalThread *pThread,                // IN, OPTIONAL
@@ -987,7 +988,7 @@ namespace CorUnix
         //
 
         virtual
-        PAL_ERROR   
+        PAL_ERROR
         ObtainHandleForObject(
             CPalThread *pThread,                // IN, OPTIONAL
             IPalObject *pObject,
@@ -1057,7 +1058,7 @@ namespace CorUnix
             DWORD dwRightsRequired,
             IPalObject **ppObject               // OUT
             ) = 0;
-        
+
     };
 
     extern IPalObjectManager *g_pObjectManager;
@@ -1336,11 +1337,11 @@ namespace CorUnix
             IFileLockController **ppLockController  // OUT
             ) = 0;
 
-        // 
+        //
         // Gets the share mode for the file
-        // (returns SHARE_MODE_NOT_INITIALIZED if file lock controller 
+        // (returns SHARE_MODE_NOT_INITIALIZED if file lock controller
         // not found)
-        // 
+        //
         virtual
         PAL_ERROR
         GetFileShareModeForFile(
@@ -1352,4 +1353,3 @@ namespace CorUnix
 }
 
 #endif // _CORUNIX_H
-

pal/src/include/pal/mutex.hpp

@@ -1,6 +1,6 @@
 //
 // Copyright (c) Microsoft. All rights reserved.
-// Licensed under the MIT license. See LICENSE file in the project root for full license information. 
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
 //
 
 /*++
@@ -51,26 +51,6 @@ namespace CorUnix
         LPCWSTR lpName,
         HANDLE *phMutex
         );
-        
-}
-
-#define SYNCSPINLOCK_F_ASYMMETRIC  1
-
-#define SPINLOCKInit(lock) (*(lock) = 0)
-#define SPINLOCKDestroy SPINLOCKInit
-
-void SPINLOCKAcquire (LONG * lock, unsigned int flags);
-void SPINLOCKRelease (LONG * lock);
-DWORD SPINLOCKTryAcquire (LONG * lock);
 
+}
 #endif //_PAL_MUTEX_H_
-
-
-
-
-
-
-
-
-
-

pal/src/include/pal/thread.hpp

@@ -778,18 +778,11 @@ TLSInitialize(
     void
     );
 
-VOID
-TLSCleanup(
-    void
-    );
-
 VOID
 WaitForEndingThreads(
     void
     );
 
-extern int free_threads_spinlock;
-
 extern PAL_ActivationFunction g_activationFunction;
 extern PAL_SafeActivationCheckFunction g_safeActivationCheckFunction;
 

pal/src/include/pal/threadsusp.hpp

@@ -100,7 +100,7 @@ namespace CorUnix
             Volatile<LONG> m_lNumThreadsSuspendedByThisThread; // number of threads that this thread has suspended; used for suspension diagnostics
 #endif
 #if DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
-            int m_nSpinlock; // thread's suspension spinlock, which is used to synchronize suspension and resumption attempts
+            CCSpinLock<false> m_nSpinlock; // thread's suspension spinlock, which is used to synchronize suspension and resumption attempts
 #else // DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
             pthread_mutex_t m_ptmSuspmutex; // thread's suspension mutex, which is used to synchronize suspension and resumption attempts
             BOOL m_fSuspmutexInitialized;
@@ -206,7 +206,7 @@ namespace CorUnix
 #endif // USE_POSIX_SEMAPHORES
 
 #if DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
-            LONG*
+            CCSpinLock<false>*
             GetSuspensionSpinlock(
                 void
                 )
@@ -371,8 +371,4 @@ namespace CorUnix
 extern const BYTE WAKEUPCODE; // use for pipe reads during self suspend.
 #endif // __cplusplus
 
-#ifdef USE_GLOBAL_LOCK_FOR_SUSPENSION
-extern LONG g_ssSuspensionLock;
-#endif
-
 #endif // _PAL_THREADSUSP_HPP

pal/src/sync/CCSpinLock.cpp

@@ -0,0 +1,118 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+#include "pal/thread.hpp"
+#include "pal/cs.hpp"
+#include "pal_assert.h"
+
+#include <sched.h>
+#include <pthread.h>
+
+template<bool shouldTrackThreadId>
+void CCSpinLock<shouldTrackThreadId>::Enter()
+{
+    size_t currentThreadId = -1, spin = 0;
+    if (shouldTrackThreadId)
+    {
+        currentThreadId = GetCurrentThreadId();
+        if (this->threadId == currentThreadId)
+        {
+            _ASSERTE(this->enterCount > 0);
+            this->enterCount++;
+            return;
+        }
+    }
+
+    while(__sync_lock_test_and_set(&lock, 1))
+    {
+        while(this->lock)
+        {
+            if (++spin > this->spinCount)
+            {
+                sched_yield();
+                spin = 0;
+            }
+        }
+    }
+
+    if (shouldTrackThreadId)
+    {
+        this->threadId = GetCurrentThreadId();
+        _ASSERTE(this->enterCount == 0);
+        this->enterCount = 1;
+    }
+}
+template void CCSpinLock<false>::Enter();
+template void CCSpinLock<true>::Enter();
+
+template<bool shouldTrackThreadId>
+bool CCSpinLock<shouldTrackThreadId>::TryEnter()
+{
+    int spin = 0;
+    bool locked = true;
+
+    while(__sync_lock_test_and_set(&lock, 1))
+    {
+        while(this->lock)
+        {
+            if (++spin > this->spinCount)
+            {
+               locked = false;
+               goto FAIL;
+            }
+        }
+    }
+
+FAIL:
+
+    if (shouldTrackThreadId)
+    {
+        size_t currentThreadId = GetCurrentThreadId();
+        if (locked)
+        {
+            this->threadId = currentThreadId;
+            _ASSERTE(this->enterCount == 0);
+            this->enterCount = 1;
+        }
+        else if (threadId == currentThreadId)
+        {
+            _ASSERTE(this->enterCount > 0);
+            this->enterCount++;
+            locked = true;
+        }
+    }
+
+    return locked;
+}
+template bool CCSpinLock<false>::TryEnter();
+template bool CCSpinLock<true>::TryEnter();
+
+template<bool shouldTrackThreadId>
+void CCSpinLock<shouldTrackThreadId>::Leave()
+{
+    if (shouldTrackThreadId)
+    {
+        _ASSERTE(threadId == GetCurrentThreadId() && "Something is terribly wrong.");
+        _ASSERTE(enterCount > 0);
+        if (--enterCount == 0)
+        {
+            this->threadId = 0;
+        }
+        else
+        {
+            return;
+        }
+    }
+    __sync_lock_release(&lock);
+}
+template void CCSpinLock<false>::Leave();
+template void CCSpinLock<true>::Leave();
+
+template<bool shouldTrackThreadId>
+bool CCSpinLock<shouldTrackThreadId>::IsLocked() const
+{
+    return this->threadId == GetCurrentThreadId();
+}
+template bool CCSpinLock<false>::IsLocked() const;
+template bool CCSpinLock<true>::IsLocked() const;

pal/src/synchobj/mutex.cpp

@@ -1,6 +1,6 @@
 //
 // Copyright (c) Microsoft. All rights reserved.
-// Licensed under the MIT license. See LICENSE file in the project root for full license information. 
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
 //
 
 /*++
@@ -13,7 +13,7 @@ Module Name:
 
 Abstract:
 
-    Implementation of mutex synchroniztion object as described in 
+    Implementation of mutex synchroniztion object as described in
     the WIN32 API
 
 Revision History:
@@ -79,13 +79,13 @@ CreateMutexA(
     HANDLE hMutex = NULL;
     CPalThread *pthr = NULL;
     PAL_ERROR palError;
-    
+
     PERF_ENTRY(CreateMutexA);
     ENTRY("CreateMutexA(lpMutexAttr=%p, bInitialOwner=%d, lpName=%p (%s)\n",
           lpMutexAttributes, bInitialOwner, lpName, lpName?lpName:"NULL");
 
     pthr = InternalGetCurrentThread();
-    
+
     if (lpName != nullptr)
     {
         ASSERT("lpName: Cross-process named objects are not supported in PAL");
@@ -110,7 +110,7 @@ CreateMutexA(
     //
 
     pthr->SetLastError(palError);
-    
+
     LOGEXIT("CreateMutexA returns HANDLE %p\n", hMutex);
     PERF_EXIT(CreateMutexA);
     return hMutex;
@@ -262,7 +262,7 @@ CorUnix::InternalCreateMutex(
     palError = g_pObjectManager->RegisterObject(
         pthr,
         pobjMutex,
-        &aotMutex, 
+        &aotMutex,
         0, // should be MUTEX_ALL_ACCESS -- currently ignored (no Win32 security)
         phMutex,
         &pobjRegisteredMutex
@@ -273,7 +273,7 @@ CorUnix::InternalCreateMutex(
     // out here to ensure that we don't try to release a reference on
     // it down the line.
     //
-    
+
     pobjMutex = NULL;
 
 InternalCreateMutexExit:
@@ -307,12 +307,12 @@ ReleaseMutex( IN HANDLE hMutex )
 {
     PAL_ERROR palError = NO_ERROR;
     CPalThread *pthr = NULL;
-    
+
     PERF_ENTRY(ReleaseMutex);
     ENTRY("ReleaseMutex(hMutex=%p)\n", hMutex);
 
     pthr = InternalGetCurrentThread();
-    
+
     palError = InternalReleaseMutex(pthr, hMutex);
 
     if (NO_ERROR != palError)
@@ -365,7 +365,7 @@ CorUnix::InternalReleaseMutex(
         ERROR("Unable to obtain object for handle %p (error %d)!\n", hMutex, palError);
         goto InternalReleaseMutexExit;
     }
-    
+
     palError = pobjMutex->GetSynchStateController(
         pthr,
         &pssc
@@ -423,9 +423,9 @@ OpenMutexA (
     HANDLE hMutex = NULL;
     CPalThread *pthr = NULL;
     PAL_ERROR palError;
-    
+
     PERF_ENTRY(OpenMutexA);
-    ENTRY("OpenMutexA(dwDesiredAccess=%#x, bInheritHandle=%d, lpName=%p (%s))\n", 
+    ENTRY("OpenMutexA(dwDesiredAccess=%#x, bInheritHandle=%d, lpName=%p (%s))\n",
           dwDesiredAccess, bInheritHandle, lpName, lpName?lpName:"NULL");
 
     pthr = InternalGetCurrentThread();
@@ -446,7 +446,7 @@ OpenMutexA (
     {
         pthr->SetLastError(palError);
     }
-        
+
     LOGEXIT("OpenMutexA returns HANDLE %p\n", hMutex);
     PERF_EXIT(OpenMutexA);
     return hMutex;
@@ -475,7 +475,7 @@ OpenMutexW(
     CPalThread *pthr = NULL;
 
     PERF_ENTRY(OpenMutexW);
-    ENTRY("OpenMutexW(dwDesiredAccess=%#x, bInheritHandle=%d, lpName=%p (%S))\n", 
+    ENTRY("OpenMutexW(dwDesiredAccess=%#x, bInheritHandle=%d, lpName=%p (%S))\n",
           dwDesiredAccess, bInheritHandle, lpName, lpName?lpName:W16_NULLSTRING);
 
     pthr = InternalGetCurrentThread();
@@ -514,7 +514,7 @@ Note:
 Parameters:
   pthr -- thread data for calling thread
   phEvent -- on success, receives the allocated mutex handle
-  
+
   See MSDN docs on OpenMutex for all other parameters.
 --*/
 
@@ -578,44 +578,6 @@ InternalOpenMutexExit:
     }
 
     LOGEXIT("InternalOpenMutex returns %d\n", palError);
-    
-    return palError;
-}
-
 
-/* Basic spinlock implementation */
-void SPINLOCKAcquire (LONG * lock, unsigned int flags)
-{
-    size_t loop_seed = 1, loop_count = 0;
-
-    if (flags & SYNCSPINLOCK_F_ASYMMETRIC)
-    {
-        loop_seed = ((size_t)pthread_self() % 10) + 1;
-    }
-    while (InterlockedCompareExchange(lock, 1, 0))
-    {
-        if (!(flags & SYNCSPINLOCK_F_ASYMMETRIC) || (++loop_count % loop_seed))
-        {
-#if PAL_IGNORE_NORMAL_THREAD_PRIORITY
-            struct timespec tsSleepTime;
-            tsSleepTime.tv_sec = 0;
-            tsSleepTime.tv_nsec = 1;
-            nanosleep(&tsSleepTime, NULL);
-#else
-            sched_yield();
-#endif 
-        }
-    }
-    
-}
-
-void SPINLOCKRelease (LONG * lock)
-{
-    *lock = 0;
-}
-
-DWORD SPINLOCKTryAcquire (LONG * lock)
-{
-    return InterlockedCompareExchange(lock, 1, 0);
-    // only returns 0 or 1.
+    return palError;
 }

pal/src/thread/pal_thread.cpp

@@ -88,7 +88,7 @@ static Volatile<CPalThread*> free_threads_list PAL_GLOBAL = NULL;
 
 /* lock to access list of free THREAD structures */
 /* NOTE: can't use a CRITICAL_SECTION here (see comment in FreeTHREAD) */
-int free_threads_spinlock = 0;
+static CCSpinLock<false> free_threads_spinlock;
 
 /* lock to access iEndingThreads counter, condition variable to signal shutdown
 thread when any remaining threads have died, and count of exiting threads that
@@ -191,6 +191,10 @@ Function:
 --*/
 BOOL TLSInitialize()
 {
+    // This will be called once and can't be called during/after another lock
+    // in place due to PAL is not yet initialized. The underlying issue here is
+    // related to whole lib/pal initialization on start.
+    free_threads_spinlock.Reset();
     /* Create the pthread key for thread objects, which we use
        for fast access to the current thread object. */
     if (pthread_key_create(&thObjKey, InternalEndCurrentThreadWrapper))
@@ -199,22 +203,9 @@ BOOL TLSInitialize()
         return FALSE;
     }
 
-    SPINLOCKInit(&free_threads_spinlock);
-
     return TRUE;
 }
 
-/*++
-Function:
-    TLSCleanup
-
-    Shutdown the TLS subsystem
---*/
-VOID TLSCleanup()
-{
-    SPINLOCKDestroy(&free_threads_spinlock);
-}
-
 /*++
 Function:
     AllocTHREAD
@@ -229,8 +220,7 @@ CPalThread* AllocTHREAD()
 {
     CPalThread* pThread = NULL;
 
-    /* Get the lock */
-    SPINLOCKAcquire(&free_threads_spinlock, 0);
+    free_threads_spinlock.Enter();
 
     pThread = free_threads_list;
     if (pThread != NULL)
@@ -238,8 +228,7 @@ CPalThread* AllocTHREAD()
         free_threads_list = pThread->GetNext();
     }
 
-    /* Release the lock */
-    SPINLOCKRelease(&free_threads_spinlock);
+    free_threads_spinlock.Leave();
 
     if (pThread == NULL)
     {
@@ -298,14 +287,12 @@ static void FreeTHREAD(CPalThread *pThread)
        Update: [TODO] PROCSuspendOtherThreads has been removed. Can this
        code be changed?*/
 
-    /* Get the lock */
-    SPINLOCKAcquire(&free_threads_spinlock, 0);
+    free_threads_spinlock.Enter();
 
     pThread->SetNext(free_threads_list);
     free_threads_list = pThread;
 
-    /* Release the lock */
-    SPINLOCKRelease(&free_threads_spinlock);
+    free_threads_spinlock.Leave();
 }
 
 
@@ -1177,12 +1164,6 @@ CorUnix::InternalSetThreadPriority(
     case THREAD_PRIORITY_NORMAL:        /* fall through */
     case THREAD_PRIORITY_BELOW_NORMAL:  /* fall through */
     case THREAD_PRIORITY_LOWEST:
-#if PAL_IGNORE_NORMAL_THREAD_PRIORITY
-        /* We aren't going to set the thread priority. Just record what it is,
-           and exit */
-        pTargetThread->m_iThreadPriority = iNewPriority;
-        goto InternalSetThreadPriorityExit;
-#endif
         break;
 
     default:

pal/src/thread/threadsusp.cpp

@@ -60,7 +60,7 @@ CONST BYTE WAKEUPCODE=0x2A;
 suspension mutex or spinlock. The downside is that it restricts us to only
 performing one suspension or resumption in the PAL at a time. */
 #ifdef USE_GLOBAL_LOCK_FOR_SUSPENSION
-static LONG g_ssSuspensionLock = 0;
+static CCSpinLock<false> g_ssSuspensionLock;
 #endif
 
 /*++
@@ -348,21 +348,19 @@ CThreadSuspensionInfo::TryAcquireSuspensionLock(
     CPalThread* pthrTarget
     )
 {
-    int iPthreadRet = 0;
 #if DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
 {
-    iPthreadRet = SPINLOCKTryAcquire(pthrTarget->suspensionInfo.GetSuspensionSpinlock());
+    return pthrTarget->suspensionInfo.GetSuspensionSpinlock()->TryEnter();
 }
 #else // DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
 {
-    iPthreadRet = pthread_mutex_trylock(pthrTarget->suspensionInfo.GetSuspensionMutex());
+    int iPthreadRet = pthread_mutex_trylock(pthrTarget->suspensionInfo.GetSuspensionMutex());
     _ASSERT_MSG(iPthreadRet == 0 || iPthreadRet == EBUSY, "pthread_mutex_trylock returned %d\n", iPthreadRet);
-}
-#endif // DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
-
     // If iPthreadRet is 0, lock acquisition was successful. Otherwise, it failed.
     return (iPthreadRet == 0);
 }
+#endif // DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
+}
 
 /*++
 Function:
@@ -380,13 +378,13 @@ CThreadSuspensionInfo::AcquireSuspensionLock(
 {
 #ifdef USE_GLOBAL_LOCK_FOR_SUSPENSION
 {
-    SPINLOCKAcquire(&g_ssSuspensionLock, 0);
+    g_ssSuspensionLock.Enter();
 }
 #else // USE_GLOBAL_LOCK_FOR_SUSPENSION
 {
     #if DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
     {
-        SPINLOCKAcquire(&pthrCurrent->suspensionInfo.m_nSpinlock, 0);
+        pthrCurrent->suspensionInfo.m_nSpinlock.Enter();
     }
     #else // DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
     {
@@ -414,13 +412,13 @@ CThreadSuspensionInfo::ReleaseSuspensionLock(
 {
 #ifdef USE_GLOBAL_LOCK_FOR_SUSPENSION
 {
-    SPINLOCKRelease(&g_ssSuspensionLock);
+    g_ssSuspensionLock.Leave();
 }
 #else // USE_GLOBAL_LOCK_FOR_SUSPENSION
 {
     #if DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
     {
-        SPINLOCKRelease(&pthrCurrent->suspensionInfo.m_nSpinlock);
+        pthrCurrent->suspensionInfo.m_nSpinlock.Leave();
     }
     #else // DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
     {
@@ -776,9 +774,7 @@ constructor.
 VOID
 CThreadSuspensionInfo::InitializeSuspensionLock()
 {
-#if DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
-    SPINLOCKInit(&m_nSpinlock);
-#else
+#if !DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX
     int iError = pthread_mutex_init(&m_ptmSuspmutex, NULL);
     if (0 != iError )
     {