From a4f5a2291e9d1f6fbd37b1d7fc0a8374a50b47cb Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 14 Jun 2019 14:34:38 +0000 Subject: [PATCH] introduce cache for sort handles to prevent from memory leak in certain scenarios --- .../System/Globalization/CompareInfo.Unix.cs | 47 ++++++++++++++++++---- .../System/Globalization/CompareInfo.Windows.cs | 3 ++ .../shared/System/Globalization/CompareInfo.cs | 6 +-- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs index e904369..40d1098 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs @@ -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 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(); + } - 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); } } diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs index a1c4a36..7f1ba8c 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs @@ -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; diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs index aa45e86..9b925f9 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs @@ -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; -- 2.7.4