From 28ca58c0e97b2273873a4eaec82f9f9cd4ded5ea Mon Sep 17 00:00:00 2001
From: Joel Linn <jl@conductive.de>
Date: Fri, 14 May 2021 00:40:41 +0200
Subject: [PATCH] [Base] Fix HighResolutionTimer

- Test was failing on Linux 5.11 and GLIBC 2.33
- `timer_t(0)` is a valid handle, so a `valid_` flag was added to guard
  destruction
- Similar behaviour on Windows was fixed as well. The invalid values for
  `HANDLE` are API dependent.
---
 src/xenia/base/threading_posix.cc | 19 ++++++++++++------
 src/xenia/base/threading_win.cc   | 32 +++++++++++++++++--------------
 2 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/src/xenia/base/threading_posix.cc b/src/xenia/base/threading_posix.cc
index 436e81cfb..c0011d603 100644
--- a/src/xenia/base/threading_posix.cc
+++ b/src/xenia/base/threading_posix.cc
@@ -155,29 +155,36 @@ bool SetTlsValue(TlsHandle handle, uintptr_t value) {
 class PosixHighResolutionTimer : public HighResolutionTimer {
  public:
   explicit PosixHighResolutionTimer(std::function<void()> callback)
-      : callback_(std::move(callback)), timer_(nullptr) {}
+      : callback_(std::move(callback)), valid_(false) {}
   ~PosixHighResolutionTimer() override {
-    if (timer_) timer_delete(timer_);
+    if (valid_) timer_delete(timer_);
   }
 
   bool Initialize(std::chrono::milliseconds period) {
+    if (valid_) {
+      // Double initialization
+      assert_always();
+      return false;
+    }
     // Create timer
     sigevent sev{};
     sev.sigev_notify = SIGEV_SIGNAL;
     sev.sigev_signo = GetSystemSignal(SignalType::kHighResolutionTimer);
     sev.sigev_value.sival_ptr = (void*)&callback_;
-    if (timer_create(CLOCK_REALTIME, &sev, &timer_) == -1) return false;
+    if (timer_create(CLOCK_MONOTONIC, &sev, &timer_) == -1) return false;
 
     // Start timer
     itimerspec its{};
     its.it_value = DurationToTimeSpec(period);
     its.it_interval = its.it_value;
-    return timer_settime(timer_, 0, &its, nullptr) != -1;
+    valid_ = timer_settime(timer_, 0, &its, nullptr) != -1;
+    return valid_;
   }
 
  private:
   std::function<void()> callback_;
   timer_t timer_;
+  bool valid_;  // all values for timer_t are legal so we need this
 };
 
 std::unique_ptr<HighResolutionTimer> HighResolutionTimer::CreateRepeating(
@@ -187,7 +194,7 @@ std::unique_ptr<HighResolutionTimer> HighResolutionTimer::CreateRepeating(
   if (!timer->Initialize(period)) {
     return nullptr;
   }
-  return std::unique_ptr<HighResolutionTimer>(timer.release());
+  return std::move(timer);
 }
 
 class PosixConditionBase {
@@ -419,7 +426,7 @@ class PosixCondition<Timer> : public PosixConditionBase {
       sev.sigev_notify = SIGEV_SIGNAL;
       sev.sigev_signo = GetSystemSignal(SignalType::kTimer);
       sev.sigev_value.sival_ptr = this;
-      if (timer_create(CLOCK_REALTIME, &sev, &timer_) == -1) return false;
+      if (timer_create(CLOCK_MONOTONIC, &sev, &timer_) == -1) return false;
     }
 
     // Start timer
diff --git a/src/xenia/base/threading_win.cc b/src/xenia/base/threading_win.cc
index e91cdf1ce..60c746e55 100644
--- a/src/xenia/base/threading_win.cc
+++ b/src/xenia/base/threading_win.cc
@@ -111,30 +111,34 @@ bool SetTlsValue(TlsHandle handle, uintptr_t value) {
 class Win32HighResolutionTimer : public HighResolutionTimer {
  public:
   Win32HighResolutionTimer(std::function<void()> callback)
-      : callback_(callback) {}
+      : callback_(std::move(callback)) {}
   ~Win32HighResolutionTimer() override {
-    if (handle_) {
+    if (valid_) {
       DeleteTimerQueueTimer(nullptr, handle_, INVALID_HANDLE_VALUE);
       handle_ = nullptr;
     }
   }
 
   bool Initialize(std::chrono::milliseconds period) {
-    return CreateTimerQueueTimer(
-               &handle_, nullptr,
-               [](PVOID param, BOOLEAN timer_or_wait_fired) {
-                 auto timer =
-                     reinterpret_cast<Win32HighResolutionTimer*>(param);
-                 timer->callback_();
-               },
-               this, 0, DWORD(period.count()), WT_EXECUTEINTIMERTHREAD)
-               ? true
-               : false;
+    if (valid_) {
+      // Double initialization
+      assert_always();
+      return false;
+    }
+    valid_ = !!CreateTimerQueueTimer(
+        &handle_, nullptr,
+        [](PVOID param, BOOLEAN timer_or_wait_fired) {
+          auto timer = reinterpret_cast<Win32HighResolutionTimer*>(param);
+          timer->callback_();
+        },
+        this, 0, DWORD(period.count()), WT_EXECUTEINTIMERTHREAD);
+    return valid_;
   }
 
  private:
-  HANDLE handle_ = nullptr;
   std::function<void()> callback_;
+  HANDLE handle_ = nullptr;
+  bool valid_ = false;  // Documentation does not state which HANDLE is invalid
 };
 
 std::unique_ptr<HighResolutionTimer> HighResolutionTimer::CreateRepeating(
@@ -143,7 +147,7 @@ std::unique_ptr<HighResolutionTimer> HighResolutionTimer::CreateRepeating(
   if (!timer->Initialize(period)) {
     return nullptr;
   }
-  return std::unique_ptr<HighResolutionTimer>(timer.release());
+  return std::move(timer);
 }
 
 template <typename T>