Skip to content
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

<chrono>: Improve exception messages when OS support is unavailable for time zones #4331

Open
StephanTLavavej opened this issue Jan 20, 2024 · 1 comment
Labels
chrono C++20 chrono enhancement Something can be improved

Comments

@StephanTLavavej
Copy link
Member

Overview

The STL's VS 2019 16.10 Changelog explained:

While the STL generally provides all features on all supported versions of Windows, leap seconds and time zones (which change over time) require OS support that was added to Windows 10. Specifically, updating the leap second database requires Windows 10 version 1809 or later, and time zones require icu.dll which is provided by Windows 10 version 1903/19H1 or later. This applies to both client and server OSes; note that Windows Server 2019 is based on Windows 10 version 1809.

When OS support is unavailable, we throw exceptions, as permitted by the Standard. However, if a programmer isn't already aware of the OS version dependency here, deducing "the end user's OS is too old" from the exception message can be an excessively mysterious process.

#1911 asks whether a fallback mechanism can be implemented for old OSes. There are no current plans to do so, and I believe that this will remain infeasible. However, it should be fairly simple to enhance the exception messages here.

Leap seconds require Windows 10 version 1809 or later

I believe that this happens in __std_tzdb_get_leap_seconds:

STL/stl/src/tzdb.cpp

Lines 597 to 601 in 442029c

LSTATUS status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, reg_key_name, 0, KEY_READ, &leap_sec_key);
if (status != ERROR_SUCCESS) {
// May not exist on older systems. Treat this as equivalent to the key existing but with no data.
return nullptr;
}

For old OSes, I don't believe that we throw an exception:

STL/stl/inc/chrono

Lines 2167 to 2175 in 442029c

size_t _Reg_post_2018_ls_size; // number of post-2018 LSs found in the registry
unique_ptr<__std_tzdb_leap_info[], _Tzdb_deleter<__std_tzdb_leap_info[]>> _Reg_ls_data{
__std_tzdb_get_leap_seconds(_Known_post_2018_ls_size, &_Reg_post_2018_ls_size)};
if (_Reg_post_2018_ls_size > _Known_post_2018_ls_size && !_Reg_ls_data) {
_Xbad_alloc(); // registry has new data, but failed to allocate storage
} else if (_Reg_post_2018_ls_size == 0 && _Reg_ls_data) {
_XGetLastError(); // allocated storage for registry data, but failed to read
}

It looks like we silently say "there's no post-2018 leap second data" for old OSes. If I'm right, then there's no exception message that needs to be enhanced here. (Leap seconds are also much less "popular" than time zones.)

Time zones require Windows 10 version 1903/19H1 or later

_Init_icu_functions is the single place where we load icu.dll:

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

The only caller is the wrapper _Acquire_icu_functions:

STL/stl/src/tzdb.cpp

Lines 101 to 108 in 442029c

[[nodiscard]] _Icu_api_level _Acquire_icu_functions() noexcept {
auto _Level = _Icu_functions._Api_level.load(_STD memory_order_acquire);
if (_Level <= _Icu_api_level::_Detecting) {
_Level = _Init_icu_functions(_Level);
}
return _Level;
}

Which in turn has 3 callsites: __std_tzdb_get_time_zones, __std_tzdb_get_current_zone, __std_tzdb_get_sys_info:

STL/stl/src/tzdb.cpp

Lines 357 to 358 in 442029c

if (_Acquire_icu_functions() < _Icu_api_level::_Has_icu_addresses) {
return _Report_error(_Info, __std_tzdb_error::_Win_error);

STL/stl/src/tzdb.cpp

Lines 460 to 461 in 442029c

if (_Acquire_icu_functions() < _Icu_api_level::_Has_icu_addresses) {
return _Report_error(_Info, __std_tzdb_error::_Win_error);

STL/stl/src/tzdb.cpp

Lines 501 to 502 in 442029c

if (_Acquire_icu_functions() < _Icu_api_level::_Has_icu_addresses) {
return _Report_error(_Info, __std_tzdb_error::_Win_error);

They all report __std_tzdb_error::_Win_error (because we haven't even loaded ICU yet; _Icu_error is for when we've loaded ICU but it fails later). They're all called via _Make_unique_tzdb_info:

STL/stl/inc/chrono

Lines 1837 to 1838 in 442029c

const auto _Info =
_Make_unique_tzdb_info<__std_tzdb_get_sys_info>(_Tz_arg.c_str(), _Tz_len, _Internal_dur.count());

auto _Info = _Make_unique_tzdb_info<__std_tzdb_get_current_zone>();

auto _Info = _Make_unique_tzdb_info<__std_tzdb_get_time_zones>();

Which has conveniently extracted the common pattern here (thanks to #4119):

STL/stl/inc/chrono

Lines 1743 to 1757 in 442029c

template <auto& _Get_tzdb_info, class... _Types>
_NODISCARD auto _Make_unique_tzdb_info(_Types&&... _Args) {
const auto _Raw_ptr = _Get_tzdb_info(_STD forward<_Types>(_Args)...);
using _Tzdb_info = remove_pointer_t<decltype(_Raw_ptr)>;
unique_ptr<_Tzdb_info, _Tzdb_deleter<_Tzdb_info>> _Info{_Raw_ptr};
if (_Info == nullptr) {
_Xbad_alloc();
} else if (_Info->_Err == __std_tzdb_error::_Win_error) {
_XGetLastError();
} else if (_Info->_Err == __std_tzdb_error::_Icu_error) {
_Xruntime_error("Internal error loading IANA database information");
}

Therefore, I believe that the _XGetLastError() is the single location that we need to modify. Unfortunately, _XGetLastError() was not designed to be extensible - it combines calling GetLastError() and throwing a system_error with it:

STL/stl/src/xonce.cpp

Lines 21 to 25 in 442029c

[[noreturn]] _CRTIMP2_PURE void __CLRCALL_PURE_OR_CDECL
_XGetLastError() { // throw system_error containing GetLastError()
error_code _Code(static_cast<int>(GetLastError()), _STD system_category());
_THROW(system_error(_Code));
}

To throw a better exception here, without try-catch-rethrow, we'd need to inject a new function into the import lib. (I think we should continue to capture GetLastError(), but provide an additional message mentioning that the OS version may be relevant.)

Suggested message

Windows versioning is complicated (but not nearly as much as MSVC versioning 😹) so for clarity I suggest a message like:

"C++20 chrono time zone support requires Windows 10 version 1903/19H1, Windows 11, Windows Server 2022, or later."

@StephanTLavavej StephanTLavavej added enhancement Something can be improved chrono C++20 chrono labels Jan 20, 2024
@stefanhige
Copy link

Just stumbled accross this problem, and I can confirm that it is not straightforward to conclude "the end user's OS is too old" from the current exception message, therefore I'd appreachiate the proposed change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chrono C++20 chrono enhancement Something can be improved
Projects
None yet
Development

No branches or pull requests

2 participants