microsoft / STL

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

`<chrono>`: Implement fallback mechanism for chrono's `[time.zone]` #1911

Open mnatsuhara opened 3 years ago

mnatsuhara commented 3 years ago

Our implementation of a WG21-P0355 relies on information shipped with the Windows OS, which is only available in a form that we can use in more recent OS versions.

The affected pieces of WG21-P0355 are pieces of [time.zone] that need access to the IANA database, including [time.zone.leap]. Our initial implementation had no alternate strategy for getting the time zone data or knowing when new leap seconds occur, but we should come back and include a fallback implementation that is usable on older OSes. We will need to do some investigation as to what these alternate strategies should exactly look like, perhaps relying on timezoneapi.h and some mapping between Windows and IANA time zone names (our initial plan before finding that we could use the ICU DLL) and a hard-coded table of leap seconds (both past and future) that will need to be maintained.

Specifically, our limitations on Windows OS versions are:

TzuChieh commented 3 years ago

On Windows 7 with Visual Studio 16.11.3, I would get a std::system_error by simply calling std::chrono::current_zone() in an empty console application (looks like the error is from get_tzdb_list()). Can the error somewhat related to this?

CaseyCarter commented 3 years ago

On Windows 7 with Visual Studio 16.11.3, I would get a std::system_error by simply calling std::chrono::current_zone() in an empty console application (looks like the error is from get_tzdb_list()). Can the error somewhat related to this?

Yes, this is definitely your problem. The current implementation needs Windows 10, and recent-ish Windows 10 at that. Failure on Windows 7 is expected until we implement some kind of fallback as the OP mentions.

TzuChieh commented 2 years ago

Thank you for the information! I will work around the issue for the time being.

mrboojum commented 1 year ago

Is there an advised work around for this issue? I would like to deploy an app build with Visual Studio 2022 17.3.0 on Windows Server 2016 but on this OS i get an error during the execution of the last line: auto tp_local = std::chrono::sys_seconds{}; mod_dt_in >> parse("%Y-%m-%d %H:%M:%S", tp_local); const auto mod_dt = std::chrono::clock_cast<std::chrono::utc_clock>(tp_local);

Is it possible to deploy the ICU.DLL to this OS and how could this be achieved? Related to this; are there any other dependencies that should be deployed when building with VS2022 17.3.0 and deploying on Server 2016 and where can I find this information?

Thanks

I've got a workaround by deploying the icu.dll next to the app (loading this dll before the code above is invoked) and deploying the ICU data files to C:\Windows\Globalization\ICU. However I don't want to put this in production (obviously).

jurihock commented 1 year ago

Our customers have industrial grade PCs with preinstalled Windows 10 LTSC Build 17763 (Version 1809), where icu.dll is known to be missing. Unfortunately, it is not always possible to upgrade the existing Windows OS to a newer version, e.g. 1903. So we are limited by this circumstance in software development and unable to use the actually standardized C++ 20 features, especially std::chono functions.

Is there any chance to provide the missing icu.dll at least via Visual C++ redistributable package?

https://learn.microsoft.com/en-us/cpp/windows/latest-supported-vc-redist

The current available redistributable is explicitly marked to support Windows Vista, 7, 8.1, 10, 11 and Visual Studio 2015, 2017, 2019, 2022.

It's a bit strange that support of even older OS versions is advertised but effectively not present.

See also C++20’s Extensions to Chrono Available in Visual Studio 2019 version 16.10

While our current implementation relies on the availability of the ICU DLL in more recent OS versions, we have plans to revisit the issue and investigate implementing a fallback for older operating systems.

doomlaur commented 1 year ago

In order to support older versions of Windows and also because of performance reasons (as explained in #2842), we were forced to write a few wrappers that handle old versions of Windows and improve performance.

In short:

  1. We have wrappers for functions like current_zone() and default_zone() that handle exceptions and return nullptr if no time zone support is available. Furthermore, we have a function called are_time_zones_available() that calls std::chrono::get_tzdb_list() which throws an exception if no time zone database is available.
  2. We have another round of wrappers around the previously mentioned functions that cache their values for performance reasons, because for our use cases we can ignore time zone changes while our software is running.
  3. We have extended std::chrono::zoned_time and overrode get_local_time() and operator=(const std::chrono::local_time<Duration> &) to handle old versions of Windows correctly. Our wrapper allows using nullptr as a time zone, in which case it instead uses mktime() (for get_local_time()) and localtime() / localtime_s() (for operator= with a local_time) to still work on older versions of Windows, in which case it always uses the local time.
  4. We made sure that our code uses our own zoned_time everywhere instead of std::chrono::zoned_time, to avoid surprises due to the fact that we cannot override virtual functions, because std::chrono::zoned_time has not been made to support inheritance.

Using inheritance with types that don't have virtual functions (because they have not been made to inherit from) is a bad idea, however, due to the circumstances, we decided that this was the best solution.

Also, caching as much as we could was critical for us, as all access to the ICU library is currently very slow for high-performance software. We even added the option of forcing legacy time handling (using localtime()/mktime()) even on modern versions of Windows whenever time zone handling is not needed, in order to further improve performance. This way we have time zones whenever we need them, and high performance otherwise.

doomlaur commented 1 year ago

I almost forgot: there's also a related issue about clock_cast in #2446, if anyone is interested. As explained there, I also wrote a small wrapper that ignores leap seconds.

zlojvavan commented 1 year ago

@mrboojum

I've got a workaround by deploying the icu.dll next to the app (loading this dll before the code above is invoked) and deploying the ICU data files to C:\Windows\Globalization\ICU

wonder what is the preferred source for getting icu.dll and other required files? thank you

zlojvavan commented 1 year ago

also had to implement some custom classes even with Hinnant's date lib last year but not for performance reason but due to different set of problems such as say failed attempts to download tz database file, parse it, requirements to use in late stages of app finalization (after tzdb instance destruction), support of boost::serialization etc.

funny that attempts to compile it with gcc revealed bug in 11.3 (something with type_traits and failing is_constructible though I did not report it, found some workaround)

now that I adopted codebase to switch between date and chrono depending on c++ and libc version and rebuilt the project as c++20 I faced the problem with missing icu.dll on my win 7 so cannot test chrono [time.zone] support at the moment

doomlaur commented 1 year ago

I've got a workaround by deploying the icu.dll next to the app (loading this dll before the code above is invoked) and deploying the ICU data files to C:\Windows\Globalization\ICU. However I don't want to put this in production (obviously).

Interesting. I was under the impression that icu.dll will only be loaded from system32, because the C++ Standard Library of Visual Studio asks for this explicitly:

const HMODULE _Icu_module = LoadLibraryExW(L"icu.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32);

The documentation for LoadLibraryExW says that the option LOAD_LIBRARY_SEARCH_SYSTEM32 does the following:

If this value is used, %windows%\system32 is searched for the DLL and its dependencies. Directories in the standard search path are not searched.

Maybe I might have misunderstood how this function actually loads DLLs?

EDIT: For the case you also want to build workarounds with icu.dll to get it working on very old versions of Windows, also note that using LoadLibraryExW with LOAD_LIBRARY_SEARCH_SYSTEM32 also has the following requirements:

Windows 7, Windows Server 2008 R2, Windows Vista and Windows Server 2008: This value requires KB2533623 to be installed. Windows Server 2003 and Windows XP: This value is not supported.

zlojvavan commented 1 year ago

@doomlaur

hello! do you happen to know the answer to my https://github.com/microsoft/STL/issues/1911#issuecomment-1396789047 ?

downloaded https://github.com/unicode-org/icu/releases/download/release-72-1/icu4c-72_1-Win64-MSVC2019.zip to no avail

also tried to install icu from vcpkg, broke it during attempts (https://github.com/microsoft/vcpkg/issues/29008) and my project (https://github.com/boostorg/filesystem/issues/273), fixed but still cannot find

also found unanswered https://stackoverflow.com/questions/71610601/how-to-build-icu-data-dll-in-windows

doomlaur commented 1 year ago

@zlojvavan I have never checked it out until now, because we built our own solution like I explained above, since we thought that it would be a very bad idea to deploy to system32. However, out of curiosity, I have now checked how the built-in icu.dll works out by using the Process Monitor. For me on the latest (fully up-to-date) Windows 11 22H2, when starting our software which uses std::chrono and time zones, icu.dll gets loaded from system32, after which all files in this path are loaded: C:\Windows\Globalization\ICU

It seems that my Windows comes with ICU 68.2.0.10. Although icuin.dll and icuuc.dll also exist in system32, they don't seem to get loaded (at least not at startup).

Now, directly to your question: I'm not sure whether you would prefer to deploy to system32, but in that case, I guess it would work to simply copy the files from a newer version of Windows. (They won't get automatic updates like newer versions of Windows will, but that's beyond the point now.)

I guess you could also build icu.dll yourself (how to build), but to my knowledge (see my comment above), you still have to deploy it to system32, in which case you could also simply copy it from a newer version of Windows. I'm currently not aware of any alternatives, although I would like to know if there are any.

zlojvavan commented 1 year ago

@doomlaur thanks for your answer. wanted to avoid building icu with "disable-renaming" option (to get rid of suffixes) and placed icu, ucuin, icuuc and few required dependence dlls from home win 10 to my win 7 system32 folder but ended up with error box about failed attempt to find kernel32.GetSystemTimePreciseAsFileTime in api-ms-win-core-sysinfo-l1-2-0.dll

doomlaur commented 1 year ago

By the way, there's a documentation page from Microsoft about ICU. There they also explain that icuuc.dll and icuin.dll were only needed for the older Windows 10 versions 1703, 1709, 1803 and 1809.

Beginning with Windows 10 1903 and Windows Server 2022, the two DLLs have been combined to a single icu.dll. Also, the old DLLs are kept in Windows for backward compatibility with software that uses the older version of ICU. However, only the newer icu.dll is used/supported by Visual Studio's C++ Standard Library for std::chrono. Furthermore, the files in C:\Windows\Globalization\ICU are also needed for ICU to work correctly.

Kemp-J commented 1 year ago

Is there a workaround for this? I've recently encountered it while performing a clock_cast in code that needs to be able to run on Server 2019. Some way to tweak the clock being casted to avoid the need for a timezone lookup? Deploying icu.dll alongside the software isn't really an option for me for various reasons. Any idea when a fix will be available?

doomlaur commented 1 year ago

I used the workaround for clock_casts explained here. Check out the last line - it avoids clock_casts by doing some manual calculations.

Kemp-J commented 1 year ago

That's useful as a stop-gap as long as we're not looking for extreme precision (luckily I don't care about time units much more precise than seconds in this case). The result of the subtraction is the type you need though so you can just do

auto last_write_time = file_time - std::chrono::file_clock::now() + std::chrono::system_clock::now();

unless I've missed something. I'm converting in the other direction (system_clock to file_clock) and this gives the correct result for me with file_clock and system_clock swapped.

doomlaur commented 1 year ago

Since Windows itself uses a precision of 100 nanoseconds, and because both the file_clock and the system_clock use this precision, and because I'm using it together with std::filesystem::last_write_time, I was able to keep the full precision of 100 nanoseconds even when using this workaround. I'm glad that it works well enough for you as well! 😄

paulharris commented 11 months ago

Yes please create a fallback for older Windows OSs. This is a very hard problem to diagnose, especially when it starts with crashing when checking file timestamps and you have to wonder if it is due to the lack of support for UTF-8 codepages etc. In the end I narrowed it down to the conversion from file_time_type to system_clock, and then narrow further to just utc_clock to system_clock...

The workaround that worked for me, for Windows Server 2016 appears to be as @doomlaur explained:

I would love to see support from Microsoft so we don't have to workaround this issue. Windows Server 16 is supposed to be in long-term support until 2027, so their new compilers should also support that OS where possible.

DJMcMayhem commented 11 months ago

I have run into this bug as well recently. It's very difficult to troubleshoot because it's not obvious that the missing icu.dll is responsible for the problem, the only noticeable symptom is that std::chrono::current_zone throws an exception. And I need to deploy this software to computers running windows server 1806.

jeff-kohn commented 7 months ago

Stumbling onto this while trying to figure out the best way to support date/time manipulation including timezones in C++ 20 for a new project. Add my vote for fixing this, IMHO it's a bug and not a feature enhancement, as this functionality is essentially useless in its current state for any real-world software that needs to ship to customers.

ingo-loehken commented 7 months ago

We are using with software for a large number of customers and also got problems with windows versions not supporting timezones or leapseconds, because windows server 2016/2019 miss support for icu, but have long-term support by Microsoft and are common Customer OS.

I wonder about, what would be the recommended way to get working on those machines/servers, because there is no redist on windows for those machines.

It is also completely unexpected and feels like "bad-old-times", where you had no c++ standard and you always need to check for each function, if the shared lib and struct with that size is avail for each of your customers.

Please provide redist packages of icu from Microsoft for all currently supported windows versions. Otherwise is just nice and we must turn back to Windows Legacy APIs (locked-in-effect), making the Standard Entities inaccessible for software development.

Just a note, about what you may do to work around (but its really not good)

The solution is really a hack, because

TheStormN commented 7 months ago

I think Microsoft should just create a redist package for ICU which can be installed on older systems, but to also offer that package as Windows update. I don't think it would be much of a dramatic change, because as it seems the hard copied icu.dll works fine on older systems.

Other approach would for the STL to support the standard zic compiled IANA database and let the user provide path to it via some kind of non-standard function as a fallback when the icu.dll is not found.

TheStormN commented 3 months ago

@CaseyCarter @StephanTLavavej Sorry to bug you again regarding this issue, but I think it is very important if you really want to state a complete C++20 support. Throwing exceptions on older Windows versions is just not something which can be expected from a commercial product, Please consider the following:

Given that many Windows versions before Windows 10 1903 still have long life ahead of them(especially server editions) I think you have to really invest into long term valuable solution as we can't just tell to our clients "update to use our software" as other solutions might not need an update and still provide the needed functionality.

Given the current state of the MS STL, I can't accept your claim of full production ready C++20 support as production ready would mean support for all Windows versions which are possible to target with Visual Studio.

Also please in the future, when you are adding some new standard library feature, do not consider to omit older but still supported versions of Windows and just tell your customers that the software would run but will throw exceptions.

mtnpke commented 1 month ago

@ingo-loehken one way to achieve your workaround would be to build a custom icu.dll from the sources at https://github.com/microsoft/icu that has a different load path for the data files. Unfortunately, the official ICU for Windows releases cannot be used, because they use multiple DLLs and not a single icu.dll. I have not tried this yet, but from a theoretical POV this should be an OK workaround that does not interfere too much with the system.

Instead, as an even more hacky (but easier to achieve) workaround, I have copied icu*.dll and the ICU data files from a recent Windows installation to an old Windows LTSB installation, and this seems to be working reasonably OK. Nothing I can generally recommend, though.

ingo-loehken commented 3 weeks ago

we did manage to support icu in older windows installations, but the import-address-table hooking technique used to redirect in older windows platforms to our own shared libs and ressources is really hacky and may fail in certain secured environments. modifying windows installations at our customers is a no-go and not allowed at-all, because it breaks integrity.