microsoft / STL

MSVC's implementation of the C++ Standard Library.
Other
10.07k stars 1.49k forks source link

<chrono>: `zoned_time` and `current_zone` ignore DST preference on Windows 11 and Server 2022 #4997

Open basilgello opened 4 days ago

basilgello commented 4 days ago

Describe the bug

Testing the migration of Kodi to std::chrono (xbmc/xbmc#18727), the bug has been reported in STL chrono implementation where time zones with DST adjustment preference grayed-out (unavailable) or disabled by user result in 1 hour difference between Kodi local time and the Windows Explorer local time.

For example, (UTC-06:00) Guadalajara, Mexico City, Monterey has no DST policy and the bug is shown on screenshot (notice top-right clock of Kodi vs bottom-right of Windows Explorer tray and the one in Date and Time Settings applet!):

00-nodst-bad

For (UTC-06:00) Central Time (US & Canada) there is a preference to apply DST adjustment or not. Turning DST adjustment ON shows the proper time in Kodi:

01-dst-good

while turning it OFF manifests the bug:

02-nodst-bad

Command-line test case

C:\Temp>type repro.cpp
#include <chrono>
#include <iostream>

int main() {
    const std::chrono::zoned_time cur_time{ std::chrono::current_zone(),
                                            std::chrono::system_clock::now() };
    std::cout << cur_time << '\n';
}

C:\Temp>cl /EHsc /W4 /WX /std:c++latest .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.41.34120 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

/std:c++latest is provided as a preview of language features from the latest C++
working draft, and we're eager to hear about bugs and suggestions for improvements.
However, note that these features are provided as-is without support, and subject
to changes or removal as the working draft evolves. See
https://go.microsoft.com/fwlink/?linkid=2045807 for details.

repro.cpp
Microsoft (R) Incremental Linker Version 14.41.34120.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:repro.exe
repro.obj

C:\Temp> REM (UTC-06:00) Guadalajara, Mexico City, Monterey - DST pref grayed out (bad!)
C:\Temp>time      
The current time is:  1:05:09.35
Enter the new time: 
C:\Temp>repro.exe
2024-10-01 02:05:13.0396870 CDT`

C:\Temp> REM (UTC-06:00) Central Time (US & Canada) - DST preference on (good!)
C:\Temp>time      
The current time is:  2:06:44.42
Enter the new time: 
C:\Temp>repro.exe
2024-10-01 02:06:47.3969896 CDT

C:\Temp> REM (UTC-06:00) Central Time (US & Canada) - DST preference off (bad!)
C:\Temp>time      
The current time is:  1:08:39.69
Enter the new time: 
C:\Temp>repro.exe
2024-10-01 02:08:42.3091382 CDT

Expected behavior

DST preferences should be honored!

STL version

C:\Temp>"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" -property catalog_productDisplayVersion
17.11.4

Additional context

None

basilgello commented 4 days ago

@HowardHinnant FYI

basilgello commented 4 days ago

As Microsoft's STL implementation is just a wrapper over ICU ucal_*, I think the bug resides deeper in ICU

@yumaoka maybe you have any ideas?

basilgello commented 4 days ago

Digging further into ICU Jira reveals https://unicode-org.atlassian.net/issues/ICU-21465 which was fixed in 68.2 and 69.1

basilgello commented 4 days ago

And https://unicode-org.atlassian.net/browse/ICU-13845

basilgello commented 4 days ago

FTR, Windows Server 2022 has icu.dll of version 64.2.0.2:

PS C:\Users\root> (Get-Item C:\Windows\System32\icu.dll).VersionInfo              

ProductVersion   FileVersion      FileName
--------------   -----------      --------
64, 2, 0, 2      64, 2, 0, 2 (... C:\Windows\System32\icu.dll

23H2 has 68.2 but the original reporter has it installed so the fix is not there:

Product Version                 : 68, 2, 0, 10
File Version                    : 68, 2, 0, 10 (WinBuild.160101.0800)

Upcoming 24H2 has 72:

File Version                : 72, 1, 0, 2 (WinBuild.160101.0800)
Product Version             : 72, 1, 0, 2
StephanTLavavej commented 3 days ago

We should check what the behavior is on 24H2; I can imagine reasons why 23H2's 68.2 might not have picked up this patch, but an upgrade of several major versions really should have.

basilgello commented 2 days ago

What bothers me more is the following: is there a chance we get the ICU fix into current Win10 as a Patch Tuesday? And how long time might it take?

Rationale is that Team Kodi must agree on minimally supported Windows version for v22 Piers. And most of Windows Kodi users are on Win10. With current implementation, Win10 1903 is an absolute minimum version possible because it is the first release that introduced ICU as a system component. But it has already reached EOL, so most practical outcome would be to get a KB-update for current Win10 edition and mark it as minimally supported for Kodi.

Another option is to postpone usage of std::chrono::time_zone to Kodi v23 and augment Howard Hinnant's date library we currently use to handle Windows timezones using EnumDynamicTimeZoneInformation / SystemTimeToTzSpecificLocalTimeEx WinAPI functions. This will lift the binding to system-shipped ICU but certainly requires some more efforts from me.