8000 Remove WinRT deps from `cctz::local_time_zone` by yukawa · Pull Request #316 · google/cctz · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove WinRT deps from cctz::local_time_zone #316

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 8, 2025
Merged

Remove WinRT deps from cctz::local_time_zone #316

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 81 additions & 110 deletions src/time_zone_lookup.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,23 @@
#endif

#if defined(_WIN32)
#include <sdkddkver.h>
// Include only when the SDK is for Windows 10 (and later), and the binary is
// targeted for Windows XP and later.
// Note: The Windows SDK added windows.globalization.h file for Windows 10, but
// MinGW did not add it until NTDDI_WIN10_NI (SDK version 10.0.22621.0).
#if ((defined(_WIN32_WINNT_WIN10) && !defined(__MINGW32__)) || \
(defined(NTDDI_WIN10_NI) && NTDDI_VERSION >= NTDDI_WIN10_NI)) && \
(_WIN32_WINNT >= _WIN32_WINNT_WINXP)
// Include only when <icu.h> is available.
// https://learn.microsoft.com/en-us/windows/win32/intl/international-components-for-unicode--icu-
// https://devblogs.microsoft.com/oldnewthing/20210527-00/?p=105255
#if defined(__has_include)
#if __has_include(<icu.h>)
#define USE_WIN32_LOCAL_TIME_ZONE
#include <roapi.h>
#include <tchar.h>
#include <wchar.h>
#include <windows.globalization.h>
#pragma push_macro("NTDDI_VERSION")
// ucal_getTimeZoneIDForWindowsID is available on Windows 10 RS3 and later.
#define NTDDI_VERSION 0x0A000004 // == NTDDI_WIN10_RS3
#include <windows.h>
#include <winstring.h>
#endif
#endif
#include <icu.h>
#include <timezoneapi.h>
#pragma pop_macro("NTDDI_VERSION")
#include <atomic>
#endif // __has_include(<icu.h>)
#endif // __has_include
#endif // _WIN32

#include <cstdlib>
#include <cstring>
Expand All @@ -60,80 +60,78 @@ namespace cctz {

namespace {
#if defined(USE_WIN32_LOCAL_TIME_ZONE)
// Calls the WinRT Calendar.GetTimeZone method to obtain the IANA ID of the
// local time zone. Returns an empty vector in case of an error.
std::string win32_local_time_zone(const HMODULE combase) {
std::string result;
const auto ro_activate_instance =
reinterpret_cast<decltype(&RoActivateInstance)>(
GetProcAddress(combase, "RoActivateInstance"));
if (!ro_activate_instance) {
return result;
}
const auto windows_create_string_reference =
reinterpret_cast<decltype(&WindowsCreateStringReference)>(
GetProcAddress(combase, "WindowsCreateStringReference"));
if (!windows_create_string_reference) {
return result;
}
const auto windows_delete_string =
reinterpret_cast<decltype(&WindowsDeleteString)>(
GetProcAddress(combase, "WindowsDeleteString"));
if (!windows_delete_string) {
return result;
}
const auto windows_get_string_raw_buffer =
reinterpret_cast<decltype(&WindowsGetStringRawBuffer)>(
GetProcAddress(combase, "WindowsGetStringRawBuffer"));
if (!windows_get_string_raw_buffer) {
return result;
// True if we have already failed to load the API.
static std::atomic_bool g_ucal_getTimeZoneIDForWindowsIDUnavailable;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to factor the initialization code into some function called once to avoid these atomics?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to factor the initialization code into some function called once to avoid these atomics?

I guess here is what you were wondering about.

std::string win32_local_time_zone() {
  // To avoid frequent DLL loadings and unloading, we obtain the API only once
  // then allow "icu.dll" to be "leaked". Given that it is a system library,
  // keeping it on memory is supposed to have no major drawback.
  const static auto ucal_getTimeZoneIDForWindowsIDFunc =
      []() -> decltype(&::ucal_getTimeZoneIDForWindowsID) {
    const HMODULE icudll =
        ::LoadLibraryExW(L"icu.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32);
    if (icudll == nullptr) {
      return nullptr;
    }
    return reinterpret_cast<decltype(&::ucal_getTimeZoneIDForWindowsID)>(
        ::GetProcAddress(icudll, "ucal_getTimeZoneIDForWindowsID"));
  }();

  DYNAMIC_TIME_ZONE_INFORMATION info = {};
  if (::GetDynamicTimeZoneInformation(&info) == TIME_ZONE_ID_INVALID) {
    return "";
  }

  ...

One concern in this approach is that thread-safe local static initialization does internally use a lock, which may cause dead lock when combined with Win32 Loader lock.

Suppose there are two running thread X and thread Y.

  1. Thread X enters DllMain, where the operating system implicitly acquires Loader lock.
  2. Thread Y enters win32_local_time_zone() then acquires the lock to initialize ucal_getTimeZoneIDForWindowsIDFunc variable. It then get blocked at LoadLibraryExW because the thread X still holds the Loader Lock.
  3. Thread X also enters win32_local_time_zone() then gets blocked because ucal_getTimeZoneIDForWindowsIDFunc is not yet fully initialized.

The current approach with std::atomic_* doesn't have the above dead lock issue, as it is just an optimistic optimization that does not try to enforce the initialization happens exactly once.

Let me know if you still prefer the thread-safe local static initialization approach. I'll update the patch accordingly.

static std::atomic<decltype(ucal_getTimeZoneIDForWindowsID)*>
g_ucal_getTimeZoneIDForWindowsIDRef;

std::string win32_local_time_zone() {
// If we have already failed to load the API, then just give up.
if (g_ucal_getTimeZoneIDForWindowsIDUnavailable.load()) {
return "";
}

// The string returned by WindowsCreateStringReference doesn't need to be
// deleted.
HSTRING calendar_class_id;
HSTRING_HEADER calendar_class_id_header;
HRESULT hr = windows_create_string_reference(
RuntimeClass_Windows_Globalization_Calendar,
sizeof(RuntimeClass_Windows_Globalization_Calendar) / sizeof(wchar_t) - 1,
&calendar_class_id_header, &calendar_class_id);
if (FAILED(hr)) {
return result;
}
auto ucal_getTimeZoneIDForWindowsIDFunc =
g_ucal_getTimeZoneIDForWindowsIDRef.load();
if (ucal_getTimeZoneIDForWindowsIDFunc == nullptr) {
// If we have already failed to load the API, then just give up.
if (g_ucal_getTimeZoneIDForWindowsIDUnavailable.load()) {
return "";
}

IInspectable* calendar;
hr = ro_activate_instance(calendar_class_id, &calendar);
if (FAILED(hr)) {
return result;
const HMODULE icudll = ::LoadLibraryExW(L"icu.dll", nullptr,
LOAD_LIBRARY_SEARCH_SYSTEM32);

if (icudll == nullptr) {
g_ucal_getTimeZoneIDForWindowsIDUnavailable.store(true);
return "";
}

ucal_getTimeZoneIDForWindowsIDFunc =
reinterpret_cast<decltype(ucal_getTimeZoneIDForWindowsID)*>(
::GetProcAddress(icudll, "ucal_getTimeZoneIDForWindowsID"));

if (ucal_getTimeZoneIDForWindowsIDFunc == nullptr) {
g_ucal_getTimeZoneIDForWindowsIDUnavailable.store(true);
return "";
}
// store-race is not a problem here, because ::GetProcAddress() returns the
// same address for the same function in the same DLL.
g_ucal_getTimeZoneIDForWindowsIDRef.store(
ucal_getTimeZoneIDForWindowsIDFunc);

// We intentionally do not call ::FreeLibrary() here to avoid frequent DLL
// loadings and unloading. As "icu.dll" is a system library, keeping it on
// memory is supposed to have no major drawback.
}

ABI::Windows::Globalization::ITimeZoneOnCalendar* time_zone;
hr = calendar->QueryInterface(IID_PPV_ARGS(&time_zone));
if (FAILED(hr)) {
calendar->Release();
return result;
DYNAMIC_TIME_ZONE_INFORMATION info = {};
if (::GetDynamicTimeZoneInformation(&info) == TIME_ZONE_ID_INVALID) {
return "";
}

HSTRING tz_hstr;
hr = time_zone->GetTimeZone(&tz_hstr);
if (SUCCEEDED(hr)) {
UINT32 wlen;
const PCWSTR tz_wstr = windows_get_string_raw_buffer(tz_hstr, &wlen);
if (tz_wstr) {
const int size =
WideCharToMultiByte(CP_UTF8, 0, tz_wstr, static_cast<int>(wlen),
nullptr, 0, nullptr, nullptr);
result.resize(static_cast<size_t>(size));
WideCharToMultiByte(CP_UTF8, 0, tz_wstr, static_cast<int>(wlen),
&result[0], size, nullptr, nullptr);
}
windows_delete_string(tz_hstr);
UChar buffer[128];
UErrorCode status = U_ZERO_ERROR;
const auto num_chars_in_buffer = ucal_getTimeZoneIDForWindowsIDFunc(
reinterpret_cast<const UChar*>(info.TimeZoneKeyName), -1, nullptr,
buffer, ARRAYSIZE(buffer), &status);
if (status != U_ZERO_ERROR || num_chars_in_buffer <= 0 ||
num_chars_in_buffer > ARRAYSIZE(buffer)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://unicode-org.github.io/icu-docs/apidoc/dev/icu4c/ucal_8h.html#ac345a8561828554c092663e541b1c7bd says the return value does not include the terminating NUL, so I think this check is actually off by 2. But it would also be an error if ucal_getTimeZoneIDForWindowsIDFunc ignored the ARRAYSIZE parameter.

Copy link
Contributor Author

Choose a reason for hiding this comm 8000 ent

The reason will be displayed to describe this comment to others. Learn more.

I locally tested several buffer size for the same local time ("Asia/Tokyo") and here is what I observed in my Windows 11 24H2 environment.

ARRAYSIZE(buffer) num_chars_in_buffer status content of buffer
12 10 U_ZERO_ERROR fully written with NUL termination
11 10 U_ZERO_ERROR fully written with NUL termination
10 10 U_STRING_NOT_TERMINATED_WARNING fully written without NUL termination
9 10 U_BUFFER_OVERFLOW_ERROR nothing is written

Note that the subsequent code expect num_chars_in_buffer to not include NUL, so I think num_chars_in_buffer > ARRAYSIZE(buffer) makes sense in this case. status != U_ZERO_ERROR check effectively covers such cases already though.

Do you have any preference on this condition? Realistically if (status != U_ZERO_ERROR) seems to be sufficient. Happy to update the patch if you have any preference.

return "";
}
time_zone->Release();
calendar->Release();
return result;

const int num_bytes_in_utf8 = ::WideCharToMultiByte(
CP_UTF8, 0, reinterpret_cast<const wchar_t*>(buffer),
static_cast<int>(num_chars_in_buffer), nullptr, 0, nullptr, nullptr);
std::string local_time_str;
local_time_str.resize(static_cast<size_t>(num_bytes_in_utf8));
::WideCharToMultiByte(CP_UTF8, 0, reinterpret_cast<const wchar_t*>(buffer),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://learn.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-widechartomultibyte

Maybe this is somewhat pedantic because it will still work, but it seems like this writes the NUL character to [local_time_str.size()-1]. Which I would think would mean you should erase the last char to make local_time_str.size() consistent with the real size.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this particular case, num_chars_in_buffer is the UTF-16 char count without NUL so it falls into the following rule.

https://learn.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-widechartomultibyte

If this parameter is set to a positive integer, the function processes exactly the specified number of characters. If the provided size does not include a terminating null character, the resulting character string is not null-terminated, and the returned length does not include this character.

So I think this is working as expected, right?

static_cast<int>(num_chars_in_buffer),
&local_time_str[0], num_bytes_in_utf8, nullptr,
nullptr);
return local_time_str;
}
#endif
#endif // USE_WIN32_LOCAL_TIME_ZONE
} // namespace

std::string time_zone::name() const {
Expand Down Expand Up @@ -255,36 +253,9 @@ time_zone local_time_zone() {
}
#endif
#if defined(USE_WIN32_LOCAL_TIME_ZONE)
// Use the WinRT Calendar class to get the local time zone. This feature is
// available on Windows 10 and later. The library is dynamically linked to
// maintain binary compatibility with Windows XP - Windows 7. On Windows 8,
// The combase.dll API functions are available but the RoActivateInstance
// call will fail for the Calendar class.
std::string winrt_tz;
const HMODULE combase =
LoadLibraryEx(_T("combase.dll"), nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32);
if (combase) {
const auto ro_initialize = reinterpret_cast<decltype(&::RoInitialize)>(
GetProcAddress(combase, "RoInitialize"));
const auto ro_uninitialize = reinterpret_cast<decltype(&::RoUninitialize)>(
GetProcAddress(combase, "RoUninitialize"));
if (ro_initialize && ro_uninitialize) {
const HRESULT hr = ro_initialize(RO_INIT_MULTITHREADED);
// RPC_E_CHANGED_MODE means that a previous RoInitialize call specified
// a different concurrency model. The WinRT runtime is initialized and
// should work for our purpose here, but we should *not* call
// RoUninitialize because it's a failure.
if (SUCCEEDED(hr) || hr == RPC_E_CHANGED_MODE) {
winrt_tz = win32_local_time_zone(combase);
if (SUCCEEDED(hr)) {
ro_uninitialize();
}
}
}
FreeLibrary(combase);
}
if (!winrt_tz.empty()) {
zone = winrt_tz.c_str();
std::string win32_tz = win32_local_time_zone();
if (!win32_tz.empty()) {
zone = win32_tz.c_str();
}
#endif

Expand Down
0