From 82d8265688a3eb13aa045b5d15723ff0ae84461e Mon Sep 17 00:00:00 2001 From: iphydf Date: Wed, 27 Dec 2023 11:54:37 +0000 Subject: [PATCH] fix: Use QueryPerformanceCounter on windows for monotonic time. This fixes time resolution issues and simplifies the code a bit. QPC can in theory jump forward in time, but in practice not by enough to matter in our use case. --- auto_tests/auto_test_support.c | 2 + toxcore/group_announce_test.cc | 2 +- toxcore/mono_time.c | 97 +++++++++------------------------- toxcore/mono_time_test.cc | 5 +- 4 files changed, 31 insertions(+), 75 deletions(-) diff --git a/auto_tests/auto_test_support.c b/auto_tests/auto_test_support.c index afc6c2cc..3a56039a 100644 --- a/auto_tests/auto_test_support.c +++ b/auto_tests/auto_test_support.c @@ -149,6 +149,8 @@ void set_mono_time_callback(AutoTox *autotox) Mono_Time *mono_time = autotox->tox->mono_time; autotox->clock = current_time_monotonic(mono_time); + ck_assert_msg(autotox->clock >= 1000, + "clock is too low (not initialised?): %lu", (unsigned long)autotox->clock); mono_time_set_current_time_callback(mono_time, nullptr, nullptr); // set to default first mono_time_set_current_time_callback(mono_time, get_state_clock_callback, &autotox->clock); } diff --git a/toxcore/group_announce_test.cc b/toxcore/group_announce_test.cc index 4f0eac6c..12995f65 100644 --- a/toxcore/group_announce_test.cc +++ b/toxcore/group_announce_test.cc @@ -9,7 +9,7 @@ namespace { struct Announces : ::testing::Test { protected: const Memory *mem_ = system_memory(); - uint64_t clock_ = 0; + uint64_t clock_ = 1000; Mono_Time *mono_time_ = nullptr; GC_Announces_List *gca_ = nullptr; GC_Announce _ann1; diff --git a/toxcore/mono_time.c b/toxcore/mono_time.c index 6603c62b..a3e5baa0 100644 --- a/toxcore/mono_time.c +++ b/toxcore/mono_time.c @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-3.0-or-later - * Copyright © 2016-2020 The TokTok team. + * Copyright © 2016-2023 The TokTok team. * Copyright © 2014 Tox project. */ #ifndef _XOPEN_SOURCE @@ -40,12 +40,6 @@ struct Mono_Time { uint64_t cur_time; uint64_t base_time; -#ifdef OS_WIN32 - /* protect `last_clock_update` and `last_clock_mono` from concurrent access */ - pthread_mutex_t last_clock_lock; - uint32_t last_clock_mono; - bool last_clock_update; -#endif #ifndef ESP_PLATFORM /* protect `time` from concurrent access */ @@ -56,42 +50,33 @@ struct Mono_Time { void *user_data; }; +static uint64_t timespec_to_u64(struct timespec clock_mono) +{ + return UINT64_C(1000) * clock_mono.tv_sec + (clock_mono.tv_nsec / UINT64_C(1000000)); +} + #ifdef OS_WIN32 non_null() static uint64_t current_time_monotonic_default(void *user_data) { - Mono_Time *const mono_time = (Mono_Time *)user_data; - - /* Must hold mono_time->last_clock_lock here */ - - /* GetTickCount provides only a 32 bit counter, but we can't use - * GetTickCount64 for backwards compatibility, so we handle wraparound - * ourselves. - */ - const uint32_t ticks = GetTickCount(); - - /* the higher 32 bits count the number of wrap arounds */ - uint64_t old_ovf = mono_time->cur_time & ~((uint64_t)UINT32_MAX); - - /* Check if time has decreased because of 32 bit wrap from GetTickCount() */ - if (ticks < mono_time->last_clock_mono) { - /* account for overflow */ - old_ovf += UINT32_MAX + UINT64_C(1); + LARGE_INTEGER freq; + LARGE_INTEGER count; + if (!QueryPerformanceFrequency(&freq)) { + return 0; } - - if (mono_time->last_clock_update) { - mono_time->last_clock_mono = ticks; - mono_time->last_clock_update = false; + if (!QueryPerformanceCounter(&count)) { + return 0; } - - /* splice the low and high bits back together */ - return old_ovf + ticks; -} -#else // !OS_WIN32 -static uint64_t timespec_to_u64(struct timespec clock_mono) -{ - return 1000ULL * clock_mono.tv_sec + (clock_mono.tv_nsec / 1000000ULL); + struct timespec sp = {0}; + sp.tv_sec = count.QuadPart / freq.QuadPart; + if (freq.QuadPart < 1000000000) { + sp.tv_nsec = (count.QuadPart % freq.QuadPart) * 1000000000 / freq.QuadPart; + } else { + sp.tv_nsec = (long)((count.QuadPart % freq.QuadPart) * (1000000000.0 / freq.QuadPart)); + } + return timespec_to_u64(sp); } +#else #ifdef __APPLE__ non_null() static uint64_t current_time_monotonic_default(void *user_data) @@ -150,19 +135,6 @@ Mono_Time *mono_time_new(const Memory *mem, mono_time_current_time_cb *current_t mono_time_set_current_time_callback(mono_time, current_time_callback, user_data); -#ifdef OS_WIN32 - - mono_time->last_clock_mono = 0; - mono_time->last_clock_update = false; - - if (pthread_mutex_init(&mono_time->last_clock_lock, nullptr) < 0) { - mem_delete(mem, mono_time->time_update_lock); - mem_delete(mem, mono_time); - return nullptr; - } - -#endif - mono_time->cur_time = 0; #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION // Maximum reproducibility. Never return time = 0. @@ -170,7 +142,7 @@ Mono_Time *mono_time_new(const Memory *mem, mono_time_current_time_cb *current_t #else // Never return time = 0 in case time() returns 0 (e.g. on microcontrollers // without battery-powered RTC or ones where NTP didn't initialise it yet). - mono_time->base_time = max_u64(1, (uint64_t)time(nullptr)) * 1000ULL - current_time_monotonic(mono_time); + mono_time->base_time = max_u64(1, (uint64_t)time(nullptr)) * UINT64_C(1000) - current_time_monotonic(mono_time); #endif mono_time_update(mono_time); @@ -183,9 +155,6 @@ void mono_time_free(const Memory *mem, Mono_Time *mono_time) if (mono_time == nullptr) { return; } -#ifdef OS_WIN32 - pthread_mutex_destroy(&mono_time->last_clock_lock); -#endif #ifndef ESP_PLATFORM pthread_rwlock_destroy(mono_time->time_update_lock); mem_delete(mem, mono_time->time_update_lock); @@ -195,16 +164,8 @@ void mono_time_free(const Memory *mem, Mono_Time *mono_time) void mono_time_update(Mono_Time *mono_time) { -#ifdef OS_WIN32 - /* we actually want to update the overflow state of mono_time here */ - pthread_mutex_lock(&mono_time->last_clock_lock); - mono_time->last_clock_update = true; -#endif const uint64_t cur_time = mono_time->base_time + mono_time->current_time_callback(mono_time->user_data); -#ifdef OS_WIN32 - pthread_mutex_unlock(&mono_time->last_clock_lock); -#endif #ifndef ESP_PLATFORM pthread_rwlock_wrlock(mono_time->time_update_lock); @@ -230,7 +191,7 @@ uint64_t mono_time_get_ms(const Mono_Time *mono_time) uint64_t mono_time_get(const Mono_Time *mono_time) { - return mono_time_get_ms(mono_time) / 1000ULL; + return mono_time_get_ms(mono_time) / UINT64_C(1000); } bool mono_time_is_timeout(const Mono_Time *mono_time, uint64_t timestamp, uint64_t timeout) @@ -257,15 +218,5 @@ void mono_time_set_current_time_callback(Mono_Time *mono_time, */ uint64_t current_time_monotonic(Mono_Time *mono_time) { - /* For WIN32 we don't want to change overflow state of mono_time here */ -#ifdef OS_WIN32 - /* We don't want to update the overflow state of mono_time here, - * but must protect against other threads */ - pthread_mutex_lock(&mono_time->last_clock_lock); -#endif - const uint64_t cur_time = mono_time->current_time_callback(mono_time->user_data); -#ifdef OS_WIN32 - pthread_mutex_unlock(&mono_time->last_clock_lock); -#endif - return cur_time; + return mono_time->current_time_callback(mono_time->user_data); } diff --git a/toxcore/mono_time_test.cc b/toxcore/mono_time_test.cc index cc667199..3a649b7b 100644 --- a/toxcore/mono_time_test.cc +++ b/toxcore/mono_time_test.cc @@ -53,11 +53,14 @@ TEST(MonoTime, IsTimeoutReal) uint64_t const start = mono_time_get(mono_time); EXPECT_FALSE(mono_time_is_timeout(mono_time, start, 5)); + const uint64_t before_sleep = mono_time_get(mono_time); std::this_thread::sleep_for(std::chrono::milliseconds(100)); mono_time_update(mono_time); + const uint64_t after_sleep = mono_time_get(mono_time); // should still not have timed out (5sec) after sleeping ~100ms - EXPECT_FALSE(mono_time_is_timeout(mono_time, start, 5)); + EXPECT_FALSE(mono_time_is_timeout(mono_time, start, 5)) + << "before sleep: " << before_sleep << ", after sleep: " << after_sleep; mono_time_free(mem, mono_time); }