-
Couldn't load subscription status.
- Fork 5.2k
Fix GetSystemTimeZone to return filtered list #121095
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix GetSystemTimeZone to return filtered list #121095
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where TimeZoneInfo.GetSystemTimeZones() on Linux/macOS could return duplicate time zones with different IDs (e.g., "UTC" and "UCT"). The root cause was that legacy time zone IDs loaded via FindSystemTimeZoneById() were being included in the results even though they weren't in the filtered zone.tab list.
Key changes:
- Modified
PopulateAllSystemTimeZones()to return a filtered dictionary instead of void - Separated the internal cache (
_systemTimeZones) from the filtered list returned to callers - Updated tests to verify no duplicates appear and removed the Linux-specific test skip
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| System/TimeZoneInfo.cs | Updated GetSystemTimeZones() to use filtered list from PopulateAllSystemTimeZones() instead of the internal cache directly |
| System/TimeZoneInfo.Win32.cs | Changed PopulateAllSystemTimeZones() to return the system time zones dictionary (no filtering needed on Windows) |
| System/TimeZoneInfo.Unix.cs | Implemented filtering logic to return only time zones from zone.tab, excluding cached legacy entries |
| System/TimeZoneInfoTests.cs | Added comprehensive test coverage and removed Linux platform skip for duplicate detection test |
|
Tagging subscribers to this area: @dotnet/area-system-datetime |
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/TimeZoneInfoTests.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Win32.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/TimeZoneInfoTests.cs
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/TimeZoneInfoTests.cs
Show resolved
Hide resolved
|
It seems the test is no longer failing now on Linux platforms either. |
|
@stephentoub just checking if you have any more feedback on this PR. Thanks! |
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/TimeZoneInfoTests.cs
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/TimeZoneInfoTests.cs
Show resolved
Hide resolved
|
/ba-g the failing tests are unrelated timing out and showing in other PRs too. |
Fixes #117422
We maintain an internal cache,
_systemTimeZones, that maps time zone IDs to their correspondingTimeZoneInfoobjects. Entries are added when eitherTimeZoneInfo.GetSystemTimeZones(...)orTimeZoneInfo.FindSystemTimeZoneById(...)is called.On Linux and macOS,
TimeZoneInfo.FindSystemTimeZoneById(...)attempts to retrieve the time zone object by reading the appropriate time zone file (for example,/usr/share/zoneinfo/America/Los_Angelesfor Pacific Time). WhenTimeZoneInfo.GetSystemTimeZones()is called, it reads the list of available time zones from the/usr/share/zoneinfo/zone.tabfile. This file provides a filtered list that excludes legacy time zones and avoids duplicate entries with different IDs.The issue arises when
TimeZoneInfo.FindSystemTimeZoneById(...)is called with a legacy time zone ID that is not listed inzone.tab. In that case, the corresponding time zone object is still created and added to the_systemTimeZonescache. Later, whenTimeZoneInfo.GetSystemTimeZones(...)is invoked, the list populated fromzone.tabmay include a duplicate time zone (same display name but different ID), such as bothUTCandUCT. This can cause problems for UI applications that display time zone names, as duplicates appear in the list.The fix ensures that
GetSystemTimeZones()returns only the filtered list derived fromzone.tab, excluding any cached entries added from legacy or non-standard sources. This approach preserves cache consistency and performance, while preventing duplicates from appearing in the final list.Windows systems are not affected by this issue, so the change will have minimal impact there aside from shared code adjustments.