Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

Commit

Permalink
TimeZoneInfo Cleanup
Browse files Browse the repository at this point in the history 
Some cleanup in CoreRT's TimeZoneInfo implementation:

- The CachedData locking is currently inconsistent. Some code is using a
private Lock object with LockHolder.Hold, while other code is locking
against the CachedData instance. This change removes the private Lock
object and just locks against the CachedData instance throughout, which
is consistent with the CoreCLR implementation.

- TimeZoneInfoComparer is being used in two places in CoreRT to sort the
list of system time zones, whereas it's only used in one place in
CoreCLR. This change refactors the code to have a shared implementation
of GetSystemTimeZones in CoreRT that handles the sorting in one place,
allowing a single Comparison<T> lambda to be used, which matches the
CoreCLR implementation. Now there is a single shared GetSystemTimeZones
implementation which calls a private static PopulateAllSystemTimeZones
method, which is implemented separately for Win32/WinRT/Unix.

- The WinRT implementation of FindSystemTimeZoneById was accessing the
cached data without any locking. Added locking.
  • Loading branch information
@justinvp
justinvp committed Dec 24, 2016
1 parent 424c07a commit 612aa1a
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 108 deletions.
6 changes: 3 additions & 3 deletions src/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
using System.Diagnostics.Contracts;
using System.Globalization;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -142,11 +143,10 @@ public static TimeZoneInfo FindSystemTimeZoneById(string id)
throw new NotImplementedException();
}

public static ReadOnlyCollection<TimeZoneInfo> GetSystemTimeZones()
private static void PopulateAllSystemTimeZones(CachedData cachedData)
{
Debug.Assert(Monitor.IsEntered(cachedData));
// UNIXTODO
if (s_cachedData._allSystemTimeZonesRead)
s_cachedData._allSystemTimeZonesRead = true;
throw new NotImplementedException();
}

Expand Down
55 changes: 8 additions & 47 deletions src/System.Private.CoreLib/src/System/TimeZoneInfo.Win32.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,61 +83,22 @@ public AdjustmentRule[] GetAdjustmentRules()
return (AdjustmentRule[])_adjustmentRules.Clone();
}

//
// GetSystemTimeZones -
//
// returns a ReadOnlyCollection<TimeZoneInfo> containing all valid TimeZone's
// from the local machine. The entries in the collection are sorted by
// 'DisplayName'.
//
// This method does *not* throw TimeZoneNotFoundException or
// InvalidTimeZoneException.
//
// <SecurityKernel Critical="True" Ring="0">
// <Asserts Name="Imperative: System.Security.PermissionSet" />
// </SecurityKernel>
public static ReadOnlyCollection<TimeZoneInfo> GetSystemTimeZones()
private static void PopulateAllSystemTimeZones(CachedData cachedData)
{
CachedData cachedData = s_cachedData;
Debug.Assert(Monitor.IsEntered(cachedData));

lock (cachedData)
using (RegistryKey reg = RegistryKey.GetBaseKey(RegistryKey.HKEY_LOCAL_MACHINE).OpenSubKey(c_timeZonesRegistryHive, writable: false))
{
if (cachedData._readOnlySystemTimeZones == null)
if (reg != null)
{
using (RegistryKey reg = RegistryKey.GetBaseKey(RegistryKey.HKEY_LOCAL_MACHINE).OpenSubKey(
c_timeZonesRegistryHive,
false
))
foreach (string keyName in reg.GetSubKeyNames())
{
if (reg != null)
{
foreach (string keyName in reg.GetSubKeyNames())
{
TimeZoneInfo value;
Exception ex;
TryGetTimeZone(keyName, false, out value, out ex, cachedData); // populate the cache
}
}
cachedData._allSystemTimeZonesRead = true;
}

List<TimeZoneInfo> list = new List<TimeZoneInfo>();
if (cachedData._systemTimeZones != null)
{
// return a collection of the cached system time zones
foreach (var pair in cachedData._systemTimeZones)
{
list.Add(pair.Value);
}
TimeZoneInfo value;
Exception ex;
TryGetTimeZone(keyName, false, out value, out ex, cachedData); // populate the cache
}

// sort and copy the TimeZoneInfo's into a ReadOnlyCollection for the user
list.Sort(new TimeZoneInfoComparer());

cachedData._readOnlySystemTimeZones = new ReadOnlyCollection<TimeZoneInfo>(list);
}
}
return cachedData._readOnlySystemTimeZones;
}

// -------- SECTION: constructors -----------------*
Expand Down
42 changes: 29 additions & 13 deletions src/System.Private.CoreLib/src/System/TimeZoneInfo.WinRT.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,20 @@ public AdjustmentRule[] GetAdjustmentRules()
return (AdjustmentRule[])_adjustmentRules.Clone();
}

public static ReadOnlyCollection<TimeZoneInfo> GetSystemTimeZones()
private static void PopulateAllSystemTimeZones(CachedData cachedData)
{
return s_cachedData.GetOrCreateReadonlySystemTimes();
Debug.Assert(Monitor.IsEntered(cachedData));

uint index = 0;
TIME_DYNAMIC_ZONE_INFORMATION tdzi;
while (Interop.mincore.EnumDynamicTimeZoneInformation(index, out tdzi) != Interop.mincore.Errors.ERROR_NO_MORE_ITEMS)
{
TimeZoneInformation timeZoneInformation = new TimeZoneInformation(tdzi);
TimeZoneInfo value;
Exception e;
TimeZoneInfoResult result = TryGetTimeZone(ref timeZoneInformation, false, out value, out e, cachedData);
index++;
}
}

public static TimeZoneInfo FindSystemTimeZoneById(string id)
Expand All @@ -76,21 +87,26 @@ public static TimeZoneInfo FindSystemTimeZoneById(string id)

TimeZoneInfo value;
CachedData cache = s_cachedData;
// Use the current cache if it exists
if (cache._systemTimeZones != null)

lock (cache)
{
if (cache._systemTimeZones.TryGetValue(id, out value))
// Use the current cache if it exists
if (cache._systemTimeZones != null)
{
return value;
if (cache._systemTimeZones.TryGetValue(id, out value))
{
return value;
}
}
}
// See if the cache was fully filled, if not, fill it then check again.
if (!cache.AreSystemTimesEnumerated)
{
cache.EnumerateSystemTimes();
if (cache._systemTimeZones.TryGetValue(id, out value))
// See if the cache was fully filled, if not, fill it then check again.
if (!cache._allSystemTimeZonesRead)
{
return value;
PopulateAllSystemTimeZones(cache);
cachedData._allSystemTimeZonesRead = true;
if (cache._systemTimeZones != null && cache._systemTimeZones.TryGetValue(id, out value))
{
return value;
}
}
}
throw new ArgumentException(String.Format(SR.Argument_TimeZoneNotFound, id));
Expand Down
97 changes: 52 additions & 45 deletions src/System.Private.CoreLib/src/System/TimeZoneInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,10 @@ internal enum TimeZoneInfoResult
private class CachedData
{
private volatile TimeZoneInfo _localTimeZone;
private Lock _lock = new Lock();

private TimeZoneInfo CreateLocal()
{
using (LockHolder.Hold(_lock))
lock (this)
{
TimeZoneInfo timeZone = _localTimeZone;
if (timeZone == null)
Expand Down Expand Up @@ -203,39 +202,6 @@ public bool Equals(OrdinalIgnoreCaseString other)
public volatile ReadOnlyCollection<TimeZoneInfo> _readOnlySystemTimeZones;
public bool _allSystemTimeZonesRead;

public void EnumerateSystemTimes()
{
LowLevelListWithIList<TimeZoneInfo> enumeratedTimeZoneList = new LowLevelListWithIList<TimeZoneInfo>();
uint index = 0;
TIME_DYNAMIC_ZONE_INFORMATION tdzi;
while (Interop.mincore.EnumDynamicTimeZoneInformation(index, out tdzi) != Interop.mincore.Errors.ERROR_NO_MORE_ITEMS)
{
TimeZoneInformation timeZoneInformation = new TimeZoneInformation(tdzi);
TimeZoneInfo value;
Exception e;
TimeZoneInfoResult result = TryGetTimeZone(ref timeZoneInformation, false, out value, out e, s_cachedData);
if (result == TimeZoneInfoResult.Success)
{
enumeratedTimeZoneList.Add(value);
}
index++;
}
TimeZoneInfo[] array = enumeratedTimeZoneList.ToArray();
Array.Sort(array, new TimeZoneInfoComparer());
_readOnlySystemTimeZones = new ReadOnlyCollection<TimeZoneInfo>(array);
}

public bool AreSystemTimesEnumerated { get { return _readOnlySystemTimeZones != null; } }

public ReadOnlyCollection<TimeZoneInfo> GetOrCreateReadonlySystemTimes()
{
if (_readOnlySystemTimeZones == null)
{
EnumerateSystemTimes();
}
return _readOnlySystemTimeZones;
}

private static TimeZoneInfo GetCurrentOneYearLocal()
{
// load the data from the OS
Expand Down Expand Up @@ -967,6 +933,57 @@ public override int GetHashCode()
return StringComparer.OrdinalIgnoreCase.GetHashCode(_id);
}

//
// GetSystemTimeZones -
//
// returns a ReadOnlyCollection<TimeZoneInfo> containing all valid TimeZone's
// from the local machine. The entries in the collection are sorted by
// 'DisplayName'.
//
// This method does *not* throw TimeZoneNotFoundException or
// InvalidTimeZoneException.
//
public static ReadOnlyCollection<TimeZoneInfo> GetSystemTimeZones()
{
CachedData cachedData = s_cachedData;

lock (cachedData)
{
if (cachedData._readOnlySystemTimeZones == null)
{
PopulateAllSystemTimeZones(cachedData);
cachedData._allSystemTimeZonesRead = true;

List<TimeZoneInfo> list;
if (cachedData._systemTimeZones != null)
{
// return a collection of the cached system time zones
list = new List<TimeZoneInfo>(cachedData._systemTimeZones.Count);
foreach (var pair in cachedData._systemTimeZones)
{
list.Add(pair.Value);
}
}
else
{
// return an empty collection
list = new List<TimeZoneInfo>();
}

// sort and copy the TimeZoneInfo's into a ReadOnlyCollection for the user
list.Sort((x, y) =>
{
// sort by BaseUtcOffset first and by DisplayName second - this is similar to the Windows Date/Time control panel
int comparison = x.BaseUtcOffset.CompareTo(y.BaseUtcOffset);
return comparison == 0 ? string.CompareOrdinal(x.DisplayName, y.DisplayName) : comparison;
});

cachedData._readOnlySystemTimeZones = new ReadOnlyCollection<TimeZoneInfo>(list);
}
}
return cachedData._readOnlySystemTimeZones;
}

//
// HasSameRules -
//
Expand Down Expand Up @@ -1913,16 +1930,6 @@ internal static Boolean UtcOffsetOutOfRange(TimeSpan offset)
}
}

private class TimeZoneInfoComparer : IComparer<TimeZoneInfo>
{
int System.Collections.Generic.IComparer<TimeZoneInfo>.Compare(TimeZoneInfo x, TimeZoneInfo y)
{
// sort by BaseUtcOffset first and by DisplayName second - this is similar to the Windows Date/Time control panel
int comparison = x.BaseUtcOffset.CompareTo(y.BaseUtcOffset);
return comparison == 0 ? String.Compare(x.DisplayName, y.DisplayName, StringComparison.Ordinal) : comparison;
}
}


// ----- SECTION: private serialization instance methods ----------------*

Expand Down

0 comments on commit 612aa1a

Please sign in to comment.