From ba0207f36231b8fc941b9c06acb921438d736dd3 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Sun, 10 Nov 2019 20:55:32 -0500 Subject: [PATCH 1/2] Remove unused argument from GetRegionDisplayName --- .../shared/System/Globalization/CultureData.Unix.cs | 2 +- .../shared/System/Globalization/CultureData.Windows.cs | 2 +- .../shared/System/Globalization/CultureData.cs | 7 ++----- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CultureData.Unix.cs b/src/System.Private.CoreLib/shared/System/Globalization/CultureData.Unix.cs index 515db543c36f..cbd42e62fd64 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CultureData.Unix.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CultureData.Unix.cs @@ -241,7 +241,7 @@ private static string GetLanguageDisplayName(string cultureName) return new CultureInfo(cultureName)._cultureData.GetLocaleInfo(cultureName, LocaleStringData.LocalizedDisplayName); } - private static string? GetRegionDisplayName(string? isoCountryCode) + private static string? GetRegionDisplayName() { // use the fallback which is to return NativeName return null; diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CultureData.Windows.cs b/src/System.Private.CoreLib/shared/System/Globalization/CultureData.Windows.cs index 3129f444529a..06b4864766c1 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CultureData.Windows.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CultureData.Windows.cs @@ -292,7 +292,7 @@ private string GetLanguageDisplayName(string cultureName) } } - private string GetRegionDisplayName(string isoCountryCode) + private string GetRegionDisplayName() { // If the current UI culture matching the OS UI language, we'll get the display name from the OS. // otherwise, we use the native name as we don't carry resources for the region display names anyway. diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CultureData.cs b/src/System.Private.CoreLib/shared/System/Globalization/CultureData.cs index c4c765f76a16..1f4bc5a70a07 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CultureData.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CultureData.cs @@ -1054,17 +1054,14 @@ internal string LocalizedCountryName { try { - _sLocalizedCountry = GetRegionDisplayName(TwoLetterISOCountryName); + _sLocalizedCountry = GetRegionDisplayName(); } catch { // do nothing. we'll fallback } - if (_sLocalizedCountry == null) - { - _sLocalizedCountry = NativeCountryName; - } + _sLocalizedCountry ??= NativeCountryName; } return _sLocalizedCountry; } From 5bb2b3b2d570765c7099de1d457268e886cd958e Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Sun, 10 Nov 2019 21:35:17 -0500 Subject: [PATCH 2/2] Address race conditions in CultureData While most of the race conditions in this file are benign, in these cases, a thread could end up returning an intermediate value due to multiple potential writes to the same field. --- .../System/Globalization/CultureData.cs | 77 +++++++++++-------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CultureData.cs b/src/System.Private.CoreLib/shared/System/Globalization/CultureData.cs index 1f4bc5a70a07..cad208b19742 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CultureData.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CultureData.cs @@ -795,17 +795,18 @@ internal string DisplayName { get { - if (_sLocalizedDisplayName == null) + string? localizedDisplayName = _sLocalizedDisplayName; + if (localizedDisplayName == null) { if (IsSupplementalCustomCulture) { if (IsNeutralCulture) { - _sLocalizedDisplayName = NativeLanguageName; + localizedDisplayName = NativeLanguageName; } else { - _sLocalizedDisplayName = NativeName; + localizedDisplayName = NativeName; } } else @@ -817,15 +818,15 @@ internal string DisplayName if (Name.Equals(ZH_CHT, StringComparison.OrdinalIgnoreCase)) { - _sLocalizedDisplayName = GetLanguageDisplayName("zh-Hant"); + localizedDisplayName = GetLanguageDisplayName("zh-Hant"); } else if (Name.Equals(ZH_CHS, StringComparison.OrdinalIgnoreCase)) { - _sLocalizedDisplayName = GetLanguageDisplayName("zh-Hans"); + localizedDisplayName = GetLanguageDisplayName("zh-Hans"); } else { - _sLocalizedDisplayName = GetLanguageDisplayName(Name); + localizedDisplayName = GetLanguageDisplayName(Name); } } catch @@ -833,13 +834,14 @@ internal string DisplayName // do nothing } } + // If it hasn't been found (Windows 8 and up), fallback to the system - if (string.IsNullOrEmpty(_sLocalizedDisplayName)) + if (string.IsNullOrEmpty(localizedDisplayName)) { // If its neutral use the language name if (IsNeutralCulture) { - _sLocalizedDisplayName = LocalizedLanguageName; + localizedDisplayName = LocalizedLanguageName; } else { @@ -851,17 +853,19 @@ internal string DisplayName ((ci = CultureInfo.GetUserDefaultCulture()) != null) && !CultureInfo.DefaultThreadCurrentUICulture.Name.Equals(ci.Name)) { - _sLocalizedDisplayName = NativeName; + localizedDisplayName = NativeName; } else { - _sLocalizedDisplayName = GetLocaleInfo(LocaleStringData.LocalizedDisplayName); + localizedDisplayName = GetLocaleInfo(LocaleStringData.LocalizedDisplayName); } } } + + _sLocalizedDisplayName = localizedDisplayName; } - return _sLocalizedDisplayName; + return localizedDisplayName; } } @@ -872,27 +876,28 @@ internal string EnglishName { get { - if (_sEnglishDisplayName == null) + string? englishDisplayName = _sEnglishDisplayName; + if (englishDisplayName == null) { // If its neutral use the language name if (IsNeutralCulture) { - _sEnglishDisplayName = EnglishLanguageName; + englishDisplayName = EnglishLanguageName; // differentiate the legacy display names switch (_sName) { case "zh-CHS": case "zh-CHT": - _sEnglishDisplayName += " Legacy"; + englishDisplayName += " Legacy"; break; } } else { - _sEnglishDisplayName = GetLocaleInfo(LocaleStringData.EnglishDisplayName); + englishDisplayName = GetLocaleInfo(LocaleStringData.EnglishDisplayName); // if it isn't found build one: - if (string.IsNullOrEmpty(_sEnglishDisplayName)) + if (string.IsNullOrEmpty(englishDisplayName)) { // Our existing names mostly look like: // "English" + "United States" -> "English (United States)" @@ -900,7 +905,7 @@ internal string EnglishName if (EnglishLanguageName[^1] == ')') { // "Azeri (Latin)" + "Azerbaijan" -> "Azeri (Latin, Azerbaijan)" - _sEnglishDisplayName = string.Concat( + englishDisplayName = string.Concat( EnglishLanguageName.AsSpan(0, _sEnglishLanguage!.Length - 1), ", ", EnglishCountryName, @@ -909,12 +914,15 @@ internal string EnglishName else { // "English" + "United States" -> "English (United States)" - _sEnglishDisplayName = EnglishLanguageName + " (" + EnglishCountryName + ")"; + englishDisplayName = EnglishLanguageName + " (" + EnglishCountryName + ")"; } } } + + _sEnglishDisplayName = englishDisplayName; } - return _sEnglishDisplayName; + + return englishDisplayName; } } @@ -925,36 +933,40 @@ internal string NativeName { get { - if (_sNativeDisplayName == null) + string? nativeDisplayName = _sNativeDisplayName; + if (nativeDisplayName == null) { // If its neutral use the language name if (IsNeutralCulture) { - _sNativeDisplayName = NativeLanguageName; + nativeDisplayName = NativeLanguageName; // differentiate the legacy display names switch (_sName) { case "zh-CHS": - _sNativeDisplayName += " \u65E7\u7248"; + nativeDisplayName += " \u65E7\u7248"; break; case "zh-CHT": - _sNativeDisplayName += " \u820A\u7248"; + nativeDisplayName += " \u820A\u7248"; break; } } else { - _sNativeDisplayName = GetLocaleInfo(LocaleStringData.NativeDisplayName); + nativeDisplayName = GetLocaleInfo(LocaleStringData.NativeDisplayName); // if it isn't found build one: - if (string.IsNullOrEmpty(_sNativeDisplayName)) + if (string.IsNullOrEmpty(nativeDisplayName)) { // These should primarily be "Deutsch (Deutschland)" type names - _sNativeDisplayName = NativeLanguageName + " (" + NativeCountryName + ")"; + nativeDisplayName = NativeLanguageName + " (" + NativeCountryName + ")"; } } + + _sNativeDisplayName = nativeDisplayName; } - return _sNativeDisplayName; + + return nativeDisplayName; } } @@ -1050,20 +1062,23 @@ internal string LocalizedCountryName { get { - if (_sLocalizedCountry == null) + string? localizedCountry = _sLocalizedCountry; + if (localizedCountry == null) { try { - _sLocalizedCountry = GetRegionDisplayName(); + localizedCountry = GetRegionDisplayName(); } catch { // do nothing. we'll fallback } - _sLocalizedCountry ??= NativeCountryName; + localizedCountry ??= NativeCountryName; + _sLocalizedCountry = localizedCountry; } - return _sLocalizedCountry; + + return localizedCountry; } }