don't acquire lock if the collator has been already created (#24973)
authorAdam Sitnik <adam.sitnik@gmail.com>
Sun, 9 Jun 2019 11:39:29 +0000 (13:39 +0200)
committerJan Kotas <jkotas@microsoft.com>
Sun, 9 Jun 2019 11:39:29 +0000 (04:39 -0700)
* don't acquire lock if the collator has been already created

* use atomic_compare_exchange_strong instead of __sync_bool_compare_and_swap

* don't try to workaround clang 5.0 bug, just use __atomic_compare_exchange_n

src/corefx/System.Globalization.Native/config.h.in
src/corefx/System.Globalization.Native/pal_collation.c

index 633bcfb..8939990 100644 (file)
@@ -1,4 +1,4 @@
 #pragma once
 
 #cmakedefine01 HAVE_UDAT_STANDALONE_SHORTER_WEEKDAYS
-#cmakedefine01 HAVE_SET_MAX_VARIABLE
+#cmakedefine01 HAVE_SET_MAX_VARIABLE
\ No newline at end of file
index 5b270e6..675d805 100644 (file)
@@ -4,7 +4,7 @@
 //
 
 #include <assert.h>
-#include <pthread.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <stdint.h>
 #include <search.h>
@@ -42,7 +42,6 @@ typedef struct { int32_t key; UCollator* UCollator; } TCollatorMap;
  */
 struct SortHandle
 {
-    pthread_mutex_t collatorsLockObject;
     UCollator* collatorsPerOption[CompareOptionsMask + 1];
 };
 
@@ -342,12 +341,6 @@ void CreateSortHandle(SortHandle** ppSortHandle)
     }
 
     memset(*ppSortHandle, 0, sizeof(SortHandle));
-
-    int result = pthread_mutex_init(&(*ppSortHandle)->collatorsLockObject, NULL);
-    if (result != 0)
-    {
-        assert(FALSE && "Unexpected pthread_mutex_init return value.");
-    }
 }
 
 ResultCode GlobalizationNative_GetSortHandle(const char* lpLocaleName, SortHandle** ppSortHandle)
@@ -366,7 +359,6 @@ ResultCode GlobalizationNative_GetSortHandle(const char* lpLocaleName, SortHandl
 
     if (U_FAILURE(err))
     {
-        pthread_mutex_destroy(&(*ppSortHandle)->collatorsLockObject);
         free(*ppSortHandle);
         (*ppSortHandle) = NULL;
     }
@@ -385,38 +377,37 @@ void GlobalizationNative_CloseSortHandle(SortHandle* pSortHandle)
         }
     }
 
-    pthread_mutex_destroy(&pSortHandle->collatorsLockObject);
-
     free(pSortHandle);
 }
 
 const UCollator* GetCollatorFromSortHandle(SortHandle* pSortHandle, int32_t options, UErrorCode* pErr)
 {
-    UCollator* pCollator;
     if (options == 0)
     {
-        pCollator = pSortHandle->collatorsPerOption[0];
+        return pSortHandle->collatorsPerOption[0];
     }
     else
     {
-        int lockResult = pthread_mutex_lock(&pSortHandle->collatorsLockObject);
-        if (lockResult != 0)
+        options &= CompareOptionsMask;
+        UCollator* pCollator = pSortHandle->collatorsPerOption[options];
+        if (pCollator != NULL)
         {
-            assert(FALSE && "Unexpected pthread_mutex_lock return value.");
+            return pCollator;
         }
 
-        options &= CompareOptionsMask;
-        pCollator = pSortHandle->collatorsPerOption[options];
-        if (pCollator == NULL)
+        pCollator = CloneCollatorWithOptions(pSortHandle->collatorsPerOption[0], options, pErr);
+        UCollator* pNull = NULL;
+
+        // we are not using the standard atomic_compare_exchange_strong to workaround bugs in clang 5.0 (https://bugs.llvm.org/show_bug.cgi?id=37457)
+        if (!__atomic_compare_exchange_n(&pSortHandle->collatorsPerOption[options], &pNull, pCollator, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST))
         {
-            pCollator = CloneCollatorWithOptions(pSortHandle->collatorsPerOption[0], options, pErr);
-            pSortHandle->collatorsPerOption[options] = pCollator;
+            ucol_close(pCollator);
+            pCollator = pSortHandle->collatorsPerOption[options];
+            assert(pCollator != NULL && "pCollator not expected to be null here.");
         }
 
-        pthread_mutex_unlock(&pSortHandle->collatorsLockObject);
+        return pCollator;
     }
-
-    return pCollator;
 }
 
 int32_t GlobalizationNative_GetSortVersion(SortHandle* pSortHandle)