[Base] Use chrono APIs for Timers

src/xenia/base/testing/threading_test.cc

@@ -11,6 +11,7 @@
 
 #include "xenia/base/threading.h"
 
+#define CATCH_CONFIG_ENABLE_CHRONO_STRINGMAKER
 #include "third_party/catch/include/catch.hpp"
 
 namespace xe {
@@ -786,7 +787,7 @@ TEST_CASE("Wait on Timer", "[timer]") {
   REQUIRE(timer);
   result = Wait(timer.get(), false, 1ms);
   REQUIRE(result == WaitResult::kTimeout);
-  REQUIRE(timer->SetOnce(1ms));  // Signals it
+  REQUIRE(timer->SetOnceAfter(1ms));  // Signals it
   result = Wait(timer.get(), false, 2ms);
   REQUIRE(result == WaitResult::kSuccess);
   result = Wait(timer.get(), false, 1ms);
@@ -797,21 +798,20 @@ TEST_CASE("Wait on Timer", "[timer]") {
   REQUIRE(timer);
   result = Wait(timer.get(), false, 1ms);
   REQUIRE(result == WaitResult::kTimeout);
-  REQUIRE(timer->SetOnce(1ms));  // Signals it
+  REQUIRE(timer->SetOnceAfter(1ms));  // Signals it
   result = Wait(timer.get(), false, 2ms);
   REQUIRE(result == WaitResult::kSuccess);
   result = Wait(timer.get(), false, 1ms);
   REQUIRE(result == WaitResult::kTimeout);  // Did reset
 
-  // TODO(bwrsandman): This test unexpectedly fails under windows
   // Test long due time
-  // timer = Timer::CreateSynchronizationTimer();
+  timer = Timer::CreateSynchronizationTimer();
-  // REQUIRE(timer->SetOnce(10s));
+  REQUIRE(timer->SetOnceAfter(10s));
-  // result = Wait(timer.get(), false, 10ms);  // Still signals under windows
+  result = Wait(timer.get(), false, 10ms);
-  // REQUIRE(result == WaitResult::kTimeout);
+  REQUIRE(result == WaitResult::kTimeout);
 
   // Test Repeating
-  REQUIRE(timer->SetRepeating(1ms, 10ms));
+  REQUIRE(timer->SetRepeatingAfter(1ms, 10ms));
   for (int i = 0; i < 10; ++i) {
     result = Wait(timer.get(), false, 20ms);
     INFO(i);
@@ -832,12 +832,12 @@ TEST_CASE("Wait on Timer", "[timer]") {
   result = Wait(timer.get(), false, 20ms);
   REQUIRE(result == WaitResult::kTimeout);
   // Cancel with SetOnce
-  REQUIRE(timer->SetRepeating(1ms, 10ms));
+  REQUIRE(timer->SetRepeatingAfter(1ms, 10ms));
   for (int i = 0; i < 10; ++i) {
     result = Wait(timer.get(), false, 20ms);
     REQUIRE(result == WaitResult::kSuccess);
   }
-  REQUIRE(timer->SetOnce(1ms));
+  REQUIRE(timer->SetOnceAfter(1ms));
   result = Wait(timer.get(), false, 20ms);
   REQUIRE(result == WaitResult::kSuccess);  // Signal from Set Once
   result = Wait(timer.get(), false, 20ms);
@@ -859,7 +859,7 @@ TEST_CASE("Wait on Multiple Timers", "[timer]") {
   REQUIRE(any_result.second == 0);
 
   // Some signaled
-  REQUIRE(timer1->SetOnce(1ms));
+  REQUIRE(timer1->SetOnceAfter(1ms));
   all_result = WaitAll({timer0.get(), timer1.get()}, false, 100ms);
   REQUIRE(all_result == WaitResult::kTimeout);
   any_result = WaitAny({timer0.get(), timer1.get()}, false, 100ms);
@@ -867,11 +867,11 @@ TEST_CASE("Wait on Multiple Timers", "[timer]") {
   REQUIRE(any_result.second == 1);
 
   // All signaled
-  REQUIRE(timer0->SetOnce(1ms));
+  REQUIRE(timer0->SetOnceAfter(1ms));
   all_result = WaitAll({timer0.get(), timer1.get()}, false, 100ms);
   REQUIRE(all_result == WaitResult::kSuccess);
-  REQUIRE(timer0->SetOnce(1ms));
+  REQUIRE(timer0->SetOnceAfter(1ms));
-  Sleep(1ms);
+  Sleep(2ms);
   any_result = WaitAny({timer0.get(), timer1.get()}, false, 100ms);
   REQUIRE(any_result.first == WaitResult::kSuccess);
   REQUIRE(any_result.second == 0);

src/xenia/base/threading.h

@@ -25,6 +25,7 @@
 #include <vector>
 
 #include "xenia/base/assert.h"
+#include "xenia/base/chrono.h"
 #include "xenia/base/literals.h"
 #include "xenia/base/platform.h"
 #include "xenia/base/threading_timer_queue.h"
@@ -338,6 +339,13 @@ class Mutant : public WaitHandle {
 // https://msdn.microsoft.com/en-us/library/windows/desktop/ms687012(v=vs.85).aspx
 class Timer : public WaitHandle {
  public:
+  // Make vtable entries for both so we can defer conversions and only do them
+  // if really necessary (let the calling code what clock it prefers). Windows
+  // kernel sync primitives will work with WinSystemClock while our own
+  // implementation works with steady_clock.
+  using WClock_ = xe::chrono::WinSystemClock;
+  using GClock_ = std::chrono::steady_clock;  // generic
+
   // Creates a timer whose state remains signaled until SetOnce() or
   // SetRepeating() is called to establish a new due time.
   static std::unique_ptr<Timer> CreateManualResetTimer();
@@ -350,25 +358,27 @@ class Timer : public WaitHandle {
   // timer is signaled and the thread that set the timer calls the optional
   // completion routine.
   // Returns true on success.
-  virtual bool SetOnce(std::chrono::nanoseconds due_time,
+  virtual bool SetOnceAfter(xe::chrono::hundrednanoseconds rel_time,
-                       std::function<void()> opt_callback = nullptr) = 0;
+                            std::function<void()> opt_callback = nullptr) = 0;
+  virtual bool SetOnceAt(WClock_::time_point due_time,
+                         std::function<void()> opt_callback = nullptr) = 0;
+  virtual bool SetOnceAt(GClock_::time_point due_time,
+                         std::function<void()> opt_callback = nullptr) = 0;
 
   // Activates the specified waitable timer. When the due time arrives, the
   // timer is signaled and the thread that set the timer calls the optional
   // completion routine. A periodic timer automatically reactivates each time
   // the period elapses, until the timer is canceled or reset.
   // Returns true on success.
-  virtual bool SetRepeating(std::chrono::nanoseconds due_time,
+  virtual bool SetRepeatingAfter(
-                            std::chrono::milliseconds period,
+      xe::chrono::hundrednanoseconds rel_time, std::chrono::milliseconds period,
-                            std::function<void()> opt_callback = nullptr) = 0;
+      std::function<void()> opt_callback = nullptr) = 0;
-  template <typename Rep, typename Period>
+  virtual bool SetRepeatingAt(WClock_::time_point due_time,
-  bool SetRepeating(std::chrono::nanoseconds due_time,
+                              std::chrono::milliseconds period,
-                    std::chrono::duration<Rep, Period> period,
+                              std::function<void()> opt_callback = nullptr) = 0;
-                    std::function<void()> opt_callback = nullptr) {
+  virtual bool SetRepeatingAt(GClock_::time_point due_time,
-    return SetRepeating(
+                              std::chrono::milliseconds period,
-        due_time, std::chrono::duration_cast<std::chrono::milliseconds>(period),
+                              std::function<void()> opt_callback = nullptr) = 0;
-        std::move(opt_callback));
-  }
 
   // Stops the timer before it can be set to the signaled state and cancels
   // outstanding callbacks. Threads performing a wait operation on the timer

src/xenia/base/threading_posix.cc

@@ -10,8 +10,10 @@
 #include "xenia/base/threading.h"
 
 #include "xenia/base/assert.h"
+#include "xenia/base/chrono_steady_cast.h"
 #include "xenia/base/delay_scheduler.h"
 #include "xenia/base/platform.h"
+#include "xenia/base/threading_timer_queue.h"
 
 #include <pthread.h>
 #include <sched.h>
@@ -133,8 +135,6 @@ inline timespec DurationToTimeSpec(
 // gdb tip, for SIG = SIGRTMIN + SignalType : handle SIG nostop
 // lldb tip, for SIG = SIGRTMIN + SignalType : process handle SIG -s false
 enum class SignalType {
-  kHighResolutionTimer,
-  kTimer,
   kThreadSuspend,
   kThreadUserCallback,
 #if XE_PLATFORM_ANDROID
@@ -430,10 +430,7 @@ template <>
 class PosixCondition<Timer> : public PosixConditionBase {
  public:
   explicit PosixCondition(bool manual_reset)
-      : timer_(nullptr),
+      : callback_(nullptr), signal_(false), manual_reset_(manual_reset) {}
-        callback_info_(nullptr),
-        signal_(false),
-        manual_reset_(manual_reset) {}
 
   virtual ~PosixCondition() { Cancel(); }
 
@@ -444,58 +441,55 @@ class PosixCondition<Timer> : public PosixConditionBase {
     return true;
   }
 
-  // TODO(bwrsandman): due_times of under 1ms deadlock under travis
+  void SetOnce(std::chrono::steady_clock::time_point due_time,
-  // TODO(joellinn): This is likely due to deadlock on mutex_ if Signal() is
+               std::function<void()> opt_callback) {
-  // called from signal_handler running in Thread A while thread A was still in
-  // Set(...) routine inside the lock
-  bool Set(std::chrono::nanoseconds due_time, std::chrono::milliseconds period,
-           std::function<void()> opt_callback = nullptr) {
     Cancel();
 
     std::lock_guard<std::mutex> lock(mutex_);
-    callback_info_ = new timer_callback_info_t(std::move(opt_callback));
+
-    callback_info_->userdata = this;
+    callback_ = std::move(opt_callback);
     signal_ = false;
-
+    wait_item_ = QueueTimerOnce(&CompletionRoutine, this, due_time);
-    // Create timer
-    sigevent sev{};
-#if XE_HAS_SIGEV_THREAD_ID
-    sev.sigev_notify = SIGEV_SIGNAL | SIGEV_THREAD_ID;
-    sev.sigev_notify_thread_id = gettid();
-#else
-    sev.sigev_notify = SIGEV_SIGNAL;
-    callback_info_->target_thread = pthread_self();
-#endif
-    sev.sigev_signo = GetSystemSignal(SignalType::kTimer);
-    sev.sigev_value.sival_ptr = callback_info_;
-    if (timer_create(CLOCK_MONOTONIC, &sev, &timer_) == -1) {
-      delete callback_info_;
-      return false;
-    }
-
-    // Start timer
-    itimerspec its{};
-    its.it_value = DurationToTimeSpec(due_time);
-    its.it_interval = DurationToTimeSpec(period);
-    return timer_settime(timer_, 0, &its, nullptr) == 0;
   }
 
-  bool Cancel() {
+  void SetRepeating(std::chrono::steady_clock::time_point due_time,
+                    std::chrono::milliseconds period,
+                    std::function<void()> opt_callback) {
+    Cancel();
+
     std::lock_guard<std::mutex> lock(mutex_);
-    bool result = true;
+
-    if (timer_) {
+    callback_ = std::move(opt_callback);
-      callback_info_->disarmed = true;
+    signal_ = false;
-      result = timer_delete(timer_) == 0;
+    wait_item_ =
-      timer_ = nullptr;
+        QueueTimerRecurring(&CompletionRoutine, this, due_time, period);
-      static_cast<void>(timers_garbage_collector_.TryScheduleAfter(
+  }
-          callback_info_, timers_garbage_collector_delay));
+
-      callback_info_ = nullptr;
+  void Cancel() {
+    if (auto wait_item = wait_item_.lock()) {
+      wait_item->Disarm();
     }
-    return result;
   }
 
   void* native_handle() const override {
-    return reinterpret_cast<void*>(timer_);
+    assert_always();
+    return nullptr;
+  }
+
+ private:
+  static void CompletionRoutine(void* userdata) {
+    assert_not_null(userdata);
+    auto timer = reinterpret_cast<PosixCondition<Timer>*>(userdata);
+    timer->Signal();
+    // As the callback may reset the timer, store local.
+    std::function<void()> callback;
+    {
+      std::lock_guard<std::mutex> lock(timer->mutex_);
+      callback = timer->callback_;
+    }
+    if (callback) {
+      callback();
+    }
   }
 
  private:
@@ -505,8 +499,8 @@ class PosixCondition<Timer> : public PosixConditionBase {
       signal_ = false;
     }
   }
-  timer_t timer_;
+  std::weak_ptr<TimerQueueWaitItem> wait_item_;
-  timer_callback_info_t* callback_info_;
+  std::function<void()> callback_;
   volatile bool signal_;
   const bool manual_reset_;
 };
@@ -1007,29 +1001,57 @@ std::unique_ptr<Mutant> Mutant::Create(bool initial_owner) {
 }
 
 class PosixTimer : public PosixConditionHandle<Timer> {
+  using WClock_ = Timer::WClock_;
+  using GClock_ = Timer::GClock_;
+
  public:
   explicit PosixTimer(bool manual_reset) : PosixConditionHandle(manual_reset) {}
   ~PosixTimer() override = default;
-  bool SetOnce(std::chrono::nanoseconds due_time,
+
-               std::function<void()> opt_callback) override {
+  bool SetOnceAfter(xe::chrono::hundrednanoseconds rel_time,
-    return handle_.Set(due_time, std::chrono::milliseconds::zero(),
+                    std::function<void()> opt_callback = nullptr) override {
-                       std::move(opt_callback));
+    return SetOnceAt(GClock_::now() + rel_time, std::move(opt_callback));
   }
-  bool SetRepeating(std::chrono::nanoseconds due_time,
+  bool SetOnceAt(WClock_::time_point due_time,
-                    std::chrono::milliseconds period,
+                 std::function<void()> opt_callback = nullptr) override {
-                    std::function<void()> opt_callback) override {
+    return SetOnceAt(date::clock_cast<GClock_>(due_time),
-    return handle_.Set(due_time, period, std::move(opt_callback));
+                     std::move(opt_callback));
+  };
+  bool SetOnceAt(GClock_::time_point due_time,
+                 std::function<void()> opt_callback = nullptr) override {
+    handle_.SetOnce(due_time, std::move(opt_callback));
+    return true;
+  }
+
+  bool SetRepeatingAfter(
+      xe::chrono::hundrednanoseconds rel_time, std::chrono::milliseconds period,
+      std::function<void()> opt_callback = nullptr) override {
+    return SetRepeatingAt(GClock_::now() + rel_time, period,
+                          std::move(opt_callback));
+  }
+  bool SetRepeatingAt(WClock_::time_point due_time,
+                      std::chrono::milliseconds period,
+                      std::function<void()> opt_callback = nullptr) override {
+    return SetRepeatingAt(date::clock_cast<GClock_>(due_time), period,
+                          std::move(opt_callback));
+  }
+  bool SetRepeatingAt(GClock_::time_point due_time,
+                      std::chrono::milliseconds period,
+                      std::function<void()> opt_callback = nullptr) override {
+    handle_.SetRepeating(due_time, period, std::move(opt_callback));
+    return true;
+  }
+  bool Cancel() override {
+    handle_.Cancel();
+    return true;
   }
-  bool Cancel() override { return handle_.Cancel(); }
 };
 
 std::unique_ptr<Timer> Timer::CreateManualResetTimer() {
-  install_signal_handler(SignalType::kTimer);
   return std::make_unique<PosixTimer>(true);
 }
 
 std::unique_ptr<Timer> Timer::CreateSynchronizationTimer() {
-  install_signal_handler(SignalType::kTimer);
   return std::make_unique<PosixTimer>(false);
 }
 
@@ -1187,53 +1209,6 @@ void set_name(const std::string_view name) {
 
 static void signal_handler(int signal, siginfo_t* info, void* /*context*/) {
   switch (GetSystemSignalType(signal)) {
-    case SignalType::kHighResolutionTimer: {
-      assert_not_null(info->si_value.sival_ptr);
-      auto timer_info =
-          reinterpret_cast<timer_callback_info_t*>(info->si_value.sival_ptr);
-      if (!timer_info->disarmed) {
-#if XE_HAS_SIGEV_THREAD_ID
-        {
-#else
-        if (pthread_self() != timer_info->target_thread) {
-          sigval info_inner{};
-          info_inner.sival_ptr = timer_info;
-          const auto queueres = pthread_sigqueue(
-              timer_info->target_thread,
-              GetSystemSignal(SignalType::kHighResolutionTimer), info_inner);
-          assert_zero(queueres);
-        } else {
-#endif
-          timer_info->callback();
-        }
-      }
-    } break;
-    case SignalType::kTimer: {
-      assert_not_null(info->si_value.sival_ptr);
-      auto timer_info =
-          reinterpret_cast<timer_callback_info_t*>(info->si_value.sival_ptr);
-      if (!timer_info->disarmed) {
-        assert_not_null(timer_info->userdata);
-        auto timer = static_cast<PosixCondition<Timer>*>(timer_info->userdata);
-#if XE_HAS_SIGEV_THREAD_ID
-        {
-#else
-        if (pthread_self() != timer_info->target_thread) {
-          sigval info_inner{};
-          info_inner.sival_ptr = timer_info;
-          const auto queueres =
-              pthread_sigqueue(timer_info->target_thread,
-                               GetSystemSignal(SignalType::kTimer), info_inner);
-          assert_zero(queueres);
-        } else {
-#endif
-          timer->Signal();
-          if (timer_info->callback) {
-            timer_info->callback();
-          }
-        }
-      }
-    } break;
     case SignalType::kThreadSuspend: {
       assert_not_null(current_thread_);
       current_thread_->WaitSuspended();

src/xenia/base/threading_win.cc

@@ -8,6 +8,7 @@
  */
 
 #include "xenia/base/assert.h"
+#include "xenia/base/chrono_steady_cast.h"
 #include "xenia/base/logging.h"
 #include "xenia/base/platform_win.h"
 #include "xenia/base/threading.h"
@@ -276,15 +277,28 @@ std::unique_ptr<Mutant> Mutant::Create(bool initial_owner) {
 }
 
 class Win32Timer : public Win32Handle<Timer> {
+  using WClock_ = Timer::WClock_;
+  using GClock_ = Timer::GClock_;
+
  public:
   explicit Win32Timer(HANDLE handle) : Win32Handle(handle) {}
   ~Win32Timer() = default;
-  bool SetOnce(std::chrono::nanoseconds due_time,
+
-               std::function<void()> opt_callback) override {
+  bool SetOnceAfter(xe::chrono::hundrednanoseconds rel_time,
+                    std::function<void()> opt_callback) override {
+    return SetOnceAt(WClock_::now() + rel_time, std::move(opt_callback));
+  }
+  bool SetOnceAt(GClock_::time_point due_time,
+                 std::function<void()> opt_callback) override {
+    return SetOnceAt(date::clock_cast<WClock_>(due_time),
+                     std::move(opt_callback));
+  }
+  bool SetOnceAt(WClock_::time_point due_time,
+                 std::function<void()> opt_callback) override {
     std::lock_guard<std::mutex> lock(mutex_);
     callback_ = std::move(opt_callback);
     LARGE_INTEGER due_time_li;
-    due_time_li.QuadPart = due_time.count() / 100;
+    due_time_li.QuadPart = WClock_::to_file_time(due_time);
     auto completion_routine =
         callback_ ? reinterpret_cast<PTIMERAPCROUTINE>(CompletionRoutine)
                   : NULL;
@@ -293,13 +307,26 @@ class Win32Timer : public Win32Handle<Timer> {
                ? true
                : false;
   }
-  bool SetRepeating(std::chrono::nanoseconds due_time,
+
-                    std::chrono::milliseconds period,
+  bool SetRepeatingAfter(
-                    std::function<void()> opt_callback) override {
+      xe::chrono::hundrednanoseconds rel_time, std::chrono::milliseconds period,
+      std::function<void()> opt_callback = nullptr) override {
+    return SetRepeatingAt(WClock_::now() + rel_time, period,
+                          std::move(opt_callback));
+  }
+  bool SetRepeatingAt(GClock_::time_point due_time,
+                      std::chrono::milliseconds period,
+                      std::function<void()> opt_callback = nullptr) {
+    return SetRepeatingAt(date::clock_cast<WClock_>(due_time), period,
+                          std::move(opt_callback));
+  }
+  bool SetRepeatingAt(WClock_::time_point due_time,
+                      std::chrono::milliseconds period,
+                      std::function<void()> opt_callback) override {
     std::lock_guard<std::mutex> lock(mutex_);
     callback_ = std::move(opt_callback);
     LARGE_INTEGER due_time_li;
-    due_time_li.QuadPart = due_time.count() / 100;
+    due_time_li.QuadPart = WClock_::to_file_time(due_time);
     auto completion_routine =
         callback_ ? reinterpret_cast<PTIMERAPCROUTINE>(CompletionRoutine)
                   : NULL;
@@ -308,6 +335,7 @@ class Win32Timer : public Win32Handle<Timer> {
                ? true
                : false;
   }
+
   bool Cancel() override {
     // Reset the callback immediately so that any completions don't call it.
     std::lock_guard<std::mutex> lock(mutex_);

src/xenia/kernel/xtimer.cc

@@ -9,7 +9,7 @@
 
 #include "xenia/kernel/xtimer.h"
 
-#include "xenia/base/clock.h"
+#include "xenia/base/chrono.h"
 #include "xenia/base/logging.h"
 #include "xenia/cpu/processor.h"
 #include "xenia/kernel/xthread.h"
@@ -40,13 +40,24 @@ void XTimer::Initialize(uint32_t timer_type) {
 
 X_STATUS XTimer::SetTimer(int64_t due_time, uint32_t period_ms,
                           uint32_t routine, uint32_t routine_arg, bool resume) {
+  using xe::chrono::WinSystemClock;
+  using xe::chrono::XSystemClock;
   // Caller is checking for STATUS_TIMER_RESUME_IGNORED.
   if (resume) {
     return X_STATUS_TIMER_RESUME_IGNORED;
   }
 
-  due_time = Clock::ScaleGuestDurationFileTime(due_time);
   period_ms = Clock::ScaleGuestDurationMillis(period_ms);
+  WinSystemClock::time_point due_tp;
+  if (due_time < 0) {
+    // Any timer implementation uses absolute times eventually, convert as early
+    // as possible for increased accuracy
+    auto after = xe::chrono::hundrednanoseconds(-due_time);
+    due_tp = date::clock_cast<WinSystemClock>(XSystemClock::now() + after);
+  } else {
+    due_tp = date::clock_cast<WinSystemClock>(
+        XSystemClock::from_file_time(due_time));
+  }
 
   // Stash routine for callback.
   callback_thread_ = XThread::GetCurrentThread();
@@ -72,12 +83,10 @@ X_STATUS XTimer::SetTimer(int64_t due_time, uint32_t period_ms,
 
   bool result;
   if (!period_ms) {
-    result = timer_->SetOnce(std::chrono::nanoseconds(due_time * 100),
+    result = timer_->SetOnceAt(due_tp, std::move(callback));
-                             std::move(callback));
   } else {
-    result = timer_->SetRepeating(std::chrono::nanoseconds(due_time * 100),
+    result = timer_->SetRepeatingAt(
-                                  std::chrono::milliseconds(period_ms),
+        due_tp, std::chrono::milliseconds(period_ms), std::move(callback));
-                                  std::move(callback));
   }
 
   return result ? X_STATUS_SUCCESS : X_STATUS_UNSUCCESSFUL;