-
Notifications
You must be signed in to change notification settings - Fork 172
Reimplement GetWindowsLocalTimeZone without icu.h
#329
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| // Copyright 2025 Google Inc. All Rights Reserved. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // https://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| #include "time_zone_name_win.h" | ||
|
|
||
| #if !defined(NOMINMAX) | ||
| #define NOMINMAX | ||
| #endif // !defined(NOMINMAX) | ||
| #include <windows.h> | ||
|
|
||
| #include <algorithm> | ||
| #include <atomic> | ||
| #include <cstdint> | ||
| #include <limits> | ||
| #include <string> | ||
| #include <type_traits> | ||
| #include <utility> | ||
|
|
||
| namespace cctz { | ||
| namespace { | ||
|
|
||
| // Define UChar as wchar_t here because Win32 APIs receive UTF-16 strings as | ||
| // wchar_t* instead of char16_t*. Using char16_t would require additional casts. | ||
| using UChar = wchar_t; | ||
|
|
||
| enum UErrorCode : std::int32_t { | ||
| U_ZERO_ERROR = 0, | ||
| U_BUFFER_OVERFLOW_ERROR = 15, | ||
| }; | ||
|
|
||
| bool U_SUCCESS(UErrorCode error) { return error <= U_ZERO_ERROR; } | ||
|
|
||
| using ucal_getTimeZoneIDForWindowsID_func = std::int32_t(__cdecl*)( | ||
| const UChar* winid, std::int32_t len, const char* region, UChar* id, | ||
| std::int32_t id_capacity, UErrorCode* status); | ||
|
|
||
| std::atomic<bool> g_unavailable; | ||
| std::atomic<ucal_getTimeZoneIDForWindowsID_func> | ||
| g_ucal_getTimeZoneIDForWindowsID; | ||
|
|
||
| template <typename T> static T AsProcAddress(HMODULE module, const char* name) { | ||
| static_assert( | ||
| std::is_pointer<T>::value && | ||
| std::is_function<typename std::remove_pointer<T>::type>::value, | ||
| "T must be a function pointer type"); | ||
| const auto proc_address = ::GetProcAddress(module, name); | ||
| return reinterpret_cast<T>(reinterpret_cast<void*>(proc_address)); | ||
| } | ||
|
|
||
| std::wstring GetSystem32Dir() { | ||
| std::wstring result; | ||
| std::uint32_t len = std::max<std::uint32_t>( | ||
| static_cast<std::uint32_t>(std::min<size_t>( | ||
| result.capacity(), std::numeric_limits<std::uint32_t>::max())), | ||
| 1); | ||
| do { | ||
| result.resize(len); | ||
| len = ::GetSystemDirectoryW(&result[0], len); | ||
| } while (len > result.size()); | ||
| result.resize(len); | ||
| return result; | ||
| } | ||
|
|
||
| ucal_getTimeZoneIDForWindowsID_func LoadIcuGetTimeZoneIDForWindowsID() { | ||
| // This function is intended to be lock free to avoid potential deadlocks | ||
| // with loader-lock taken inside LoadLibraryW. As LoadLibraryW and | ||
| // GetProcAddress are idempotent unless the DLL is unloaded, we just need to | ||
| // make sure global variables are read/written atomically, where | ||
| // memory_order_relaxed is also acceptable. | ||
|
|
||
| if (g_unavailable.load(std::memory_order_relaxed)) { | ||
| return nullptr; | ||
| } | ||
|
|
||
| { | ||
| const auto ucal_getTimeZoneIDForWindowsIDRef = | ||
| g_ucal_getTimeZoneIDForWindowsID.load(std::memory_order_relaxed); | ||
| if (ucal_getTimeZoneIDForWindowsIDRef != nullptr) { | ||
| return ucal_getTimeZoneIDForWindowsIDRef; | ||
| } | ||
| } | ||
|
|
||
| const std::wstring system32_dir = GetSystem32Dir(); | ||
| if (system32_dir.empty()) { | ||
| g_unavailable.store(true, std::memory_order_relaxed); | ||
| return nullptr; | ||
| } | ||
|
|
||
| // Here LoadLibraryExW(L"icu.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32) does | ||
| // not work if "icu.dll" is already loaded from somewhere other than the | ||
| // system32 directory. Specifying the full path with LoadLibraryW is more | ||
| // reliable. | ||
| const std::wstring icu_dll_path = system32_dir + L"\\icu.dll"; | ||
| const HMODULE icu_dll = ::LoadLibraryW(icu_dll_path.c_str()); | ||
| if (icu_dll == nullptr) { | ||
| g_unavailable.store(true, std::memory_order_relaxed); | ||
| return nullptr; | ||
| } | ||
|
|
||
| const auto ucal_getTimeZoneIDForWindowsIDRef = | ||
| AsProcAddress<ucal_getTimeZoneIDForWindowsID_func>( | ||
| icu_dll, "ucal_getTimeZoneIDForWindowsID"); | ||
| if (ucal_getTimeZoneIDForWindowsIDRef != nullptr) { | ||
| g_unavailable.store(true, std::memory_order_relaxed); | ||
| return nullptr; | ||
| } | ||
|
|
||
| g_ucal_getTimeZoneIDForWindowsID.store(ucal_getTimeZoneIDForWindowsIDRef, | ||
| std::memory_order_relaxed); | ||
|
|
||
| return ucal_getTimeZoneIDForWindowsIDRef; | ||
| } | ||
|
|
||
| // Convert wchar_t array (UTF-16) to UTF-8 string | ||
| std::string Utf16ToUtf8(const wchar_t* ptr, size_t size) { | ||
| if (size > std::numeric_limits<int>::max()) { | ||
| return std::string(); | ||
| } | ||
| const int chars_len = static_cast<int>(size); | ||
| std::string result; | ||
| std::int32_t len = std::max<std::int32_t>( | ||
| static_cast<std::int32_t>(std::min<size_t>( | ||
| result.capacity(), std::numeric_limits<std::int32_t>::max())), | ||
| 1); | ||
| do { | ||
| result.resize(len); | ||
| // TODO: Switch to std::string::data() when we require C++17 or higher. | ||
| len = ::WideCharToMultiByte(CP_UTF8, WC_ERR_INVALID_CHARS, ptr, chars_len, | ||
| &result[0], len, nullptr, nullptr); | ||
| } while (len > result.size()); | ||
| result.resize(len); | ||
| return result; | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| std::string GetWindowsLocalTimeZone() { | ||
| const auto getTimeZoneIDForWindowsID = LoadIcuGetTimeZoneIDForWindowsID(); | ||
| if (getTimeZoneIDForWindowsID == nullptr) { | ||
| return std::string(); | ||
| } | ||
|
|
||
| DYNAMIC_TIME_ZONE_INFORMATION info = {}; | ||
| if (::GetDynamicTimeZoneInformation(&info) == TIME_ZONE_ID_INVALID) { | ||
| return std::string(); | ||
| } | ||
|
|
||
| std::wstring result; | ||
| std::int32_t len = std::max<std::int32_t>( | ||
| static_cast<std::int32_t>(std::min<size_t>( | ||
| result.capacity(), std::numeric_limits<std::int32_t>::max())), | ||
| 1); | ||
| for (;;) { | ||
| UErrorCode status = U_ZERO_ERROR; | ||
| result.resize(len); | ||
| len = getTimeZoneIDForWindowsID(info.TimeZoneKeyName, -1, nullptr, | ||
| &result[0], len, &status); | ||
| if (U_SUCCESS(status)) { | ||
| return Utf16ToUtf8(result.data(), len); | ||
| } | ||
| if (status != U_BUFFER_OVERFLOW_ERROR) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would probably also fail if Indeed, that test should come before the Alternatively, the initial
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Initialized
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe that using
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| return std::string(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } // namespace cctz | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| // Copyright 2025 Google Inc. All Rights Reserved. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // https://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| #ifndef CCTZ_TIME_ZONE_NAME_WIN_H_ | ||
| #define CCTZ_TIME_ZONE_NAME_WIN_H_ | ||
|
|
||
| #include <string> | ||
|
|
||
| namespace cctz { | ||
|
|
||
| // Returns the local time zone ID in IANA format (e.g. "America/Los_Angeles"), | ||
| // or the empty string on failure. Not supported on Windows 10 1809 and earlier, | ||
| // where "icu.dll" is not available in the System32 directory. | ||
| std::string GetWindowsLocalTimeZone(); | ||
yukawa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| } // namespace cctz | ||
|
|
||
| #endif // CCTZ_TIME_ZONE_NAME_WIN_H_ | ||
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.
I don't know why "lock-free" is a goal here. What deadlocks are possible if we only call
LoadLibraryW()at most once?That is, for my money,
GetWindowsLocalTimeZone()should just say something like ...No need for atomic anything.
Uh oh!
There was an error while loading. Please reload this page.
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.
Win32 is an interesting environment, where things like
LoadLibraryW(),GetProcAddress(), andDllMain()share the same lock.As explained in #316 (comment), I guess deadlocks are still possible in the following scenario.
Suppose there are two running thread X and thread Y.
DllMain, where the operating system implicitly acquires Loader lock.static const auto getTimeZoneIDForWindowsIDvariable. It then get blocked atLoadLibraryWbecause the thread X still holds the Loader Lock.static const auto getTimeZoneIDForWindowsIDthen gets blocked because it 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 exactly-once initialization.Let me know if I misunderstood your question.
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.
@devbww Let me know if there is any remaining concerns on this.
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.
I guess my question boils down to situation 3. Why/how would a thread inside
DllMainbe calling CCTZ'sGetWindowsLocalTimeZone()?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.
In my observation, people do not directly call
cctz::local_time_zone()but end up calling it through higher level libraries that internally use CCTZ. A most notable example would be abseil and protobuf.For instance, just calling
absl::InitializeLog()ends up internally callingGetWindowsLocalTimeZone().https://github.com/abseil/abseil-cpp/blob/ff4395895672c03fb544be00e3d66ebada1fcc47/absl/log/initialize.cc#L35
Other commonly-used methods that internally trigger
GetWindowsLocalTimeZone()would be:absl::AbslStringify(absl::Time)absl::FormatTime(absl::Time)In the real world
DllMainimplementations, it becomes extremely difficult for us to see if methods likeabsl::InitializeLog()andabsl::FormatTime(absl::Time)are indirectly called or not. Here is an excerpt from Chromium code base.https://github.com/chromium/chromium/blob/c56a646a9557ea1d54bc4ab7b9be2f1320a47e2d/chrome/credential_provider/gaiacp/gaia_credential_provider_module.cc#L148-L204
I actually don't know whether methods like
absl::FormatTime(absl::Time)andabsl::FormatTime(absl::Time)are called or not in the above code, but the fact it's hard to see is already a good reason to make it safe in the first place, I believe.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.
OK, I'm going to defer to your expertise here.
Still, it seems to me that it is a fundamental flaw having interfaces that take a lock, while having others that can call out to arbitrary code while holding the same lock. If that didn't happen everyone would be better off, and we wouldn't have to perform these dances.