introduce cache for sort handles to prevent from memory leak in certain scenarios
authorAdam Sitnik <adam.sitnik@microsoft.com>
Fri, 14 Jun 2019 14:34:38 +0000 (14:34 +0000)
committerAdam Sitnik <adam.sitnik@microsoft.com>
Fri, 14 Jun 2019 14:34:38 +0000 (14:34 +0000)
src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs
src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs
src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs

index e904369..40d1098 100644 (file)
@@ -3,10 +3,12 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Buffers;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Security;
+using System.Threading;
 
 using Internal.Runtime.CompilerServices;
 
@@ -15,8 +17,16 @@ namespace System.Globalization
     public partial class CompareInfo
     {
         [NonSerialized]
+        private IntPtr _sortHandle;
+
+        [NonSerialized]
         private bool _isAsciiEqualityOrdinal;
 
+        // in most scenarios there is a limited number of cultures with limited number of sort options
+        // so caching the sort handles and not freeing them is OK, see https://github.com/dotnet/coreclr/pull/25117 for more
+        [NonSerialized]
+        private static Dictionary<string, IntPtr> s_sortNameToSortHandleCache;
+
         private void InitSort(CultureInfo culture)
         {
             _sortName = culture.SortName;
@@ -27,17 +37,38 @@ namespace System.Globalization
             }
             else
             {
-                Interop.Globalization.ResultCode resultCode = Interop.Globalization.GetSortHandle(GetNullTerminatedUtf8String(_sortName), out _sortHandle);
-                if (resultCode != Interop.Globalization.ResultCode.Success)
-                {
-                    Interop.Globalization.CloseSortHandle(_sortHandle);
+                _isAsciiEqualityOrdinal = (_sortName == "en-US" || _sortName == "");
 
-                    if (resultCode == Interop.Globalization.ResultCode.OutOfMemory)
-                        throw new OutOfMemoryException();
+                lock (_lock)
+                {
+                    if (s_sortNameToSortHandleCache == null)
+                    {
+                        s_sortNameToSortHandleCache = new Dictionary<string, IntPtr>();
+                    }
 
-                    throw new ExternalException(SR.Arg_ExternalException);
+                    if (!s_sortNameToSortHandleCache.TryGetValue(_sortName, out _sortHandle))
+                    {
+                        s_sortNameToSortHandleCache.Add(_sortName, (_sortHandle = GetSortHandle(_sortName)));
+                    }
                 }
-                _isAsciiEqualityOrdinal = (_sortName == "en-US" || _sortName == "");
+            }
+        }
+
+        private static IntPtr GetSortHandle(string sortName)
+        {
+            var resultCode = Interop.Globalization.GetSortHandle(GetNullTerminatedUtf8String(sortName), out IntPtr sortHandle);
+            if (resultCode == Interop.Globalization.ResultCode.Success)
+            {
+                return sortHandle;
+            }
+            else
+            {
+                Interop.Globalization.CloseSortHandle(sortHandle);
+
+                if (resultCode == Interop.Globalization.ResultCode.OutOfMemory)
+                    throw new OutOfMemoryException();
+
+                throw new ExternalException(SR.Arg_ExternalException);
             }
         }
 
index a1c4a36..7f1ba8c 100644 (file)
@@ -427,6 +427,9 @@ namespace System.Globalization
         }
 
         // PAL ends here
+        [NonSerialized]
+        private IntPtr _sortHandle;
+
         private const uint LCMAP_SORTKEY = 0x00000400;
         private const uint LCMAP_HASH    = 0x00040000;
 
index aa45e86..9b925f9 100644 (file)
@@ -39,6 +39,9 @@ namespace System.Globalization
             ~(CompareOptions.IgnoreCase | CompareOptions.IgnoreSymbols | CompareOptions.IgnoreNonSpace |
               CompareOptions.IgnoreWidth | CompareOptions.IgnoreKanaType | CompareOptions.StringSort);
 
+        [NonSerialized]
+        private static readonly object _lock = new object(); // must be initialized before Invariant field
+
         // Cache the invariant CompareInfo
         internal static readonly CompareInfo Invariant = CultureInfo.InvariantCulture.CompareInfo;
 
@@ -57,9 +60,6 @@ namespace System.Globalization
 
         private int culture; // Do not rename (binary serialization). The fields sole purpose is to support Desktop serialization.
 
-        [NonSerialized]
-        private IntPtr _sortHandle;
-
         internal CompareInfo(CultureInfo culture)
         {
             m_name = culture._name;