fix: support non-unique handles for timers (#71)

bytecodealliance · Jun 26, 2024 · d7c6348 · d7c6348
1 parent af6c533
commit d7c6348
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 24 deletions.
diff --git a/builtins/web/timers.cpp b/builtins/web/timers.cpp
@@ -12,14 +12,27 @@
 #define S_TO_NS(s) ((s) * 1000000000)
 #define NS_TO_MS(ns) ((ns) / 1000000)
 
+namespace {
+
+class TimersMap {
+public:
+  std::map<int32_t, api::AsyncTask*> timers_ = {};
+  int32_t next_timer_id = 1;
+  void trace(JSTracer *trc) {
+    for (auto& [id, timer] : timers_) {
+      timer->trace(trc);
+    }
+  }
+};
+
+}
+
+static PersistentRooted<js::UniquePtr<TimersMap>> TIMERS_MAP;
 static api::Engine *ENGINE;
 
 class TimerTask final : public api::AsyncTask {
   using TimerArgumentsVector = std::vector<JS::Heap<JS::Value>>;
 
-  static std::map<int32_t, PollableHandle> timer_ids_;
-  static int32_t next_timer_id;
-
   int32_t timer_id_;
   int64_t delay_;
   int64_t deadline_;
@@ -40,8 +53,8 @@ class TimerTask final : public api::AsyncTask {
     }
 
     handle_ = host_api::MonotonicClock::subscribe(deadline_, true);
-    timer_id_ = next_timer_id++;
-    timer_ids_.emplace(timer_id_, handle_);
+    timer_id_ = TIMERS_MAP->next_timer_id++;
+    TIMERS_MAP->timers_.emplace(timer_id_, this);
   }
 
   [[nodiscard]] bool run(api::Engine *engine) override {
@@ -68,17 +81,21 @@ class TimerTask final : public api::AsyncTask {
       host_api::MonotonicClock::unsubscribe(handle_);
     }
 
-    if (repeat_ && timer_ids_.contains(timer_id_)) {
-      deadline_ = host_api::MonotonicClock::now() + delay_;
-      handle_ = host_api::MonotonicClock::subscribe(deadline_, true);
-      engine->queue_async_task(this);
+    if (TIMERS_MAP->timers_.contains(timer_id_)) {
+      if (repeat_) {
+        deadline_ = host_api::MonotonicClock::now() + delay_;
+        handle_ = host_api::MonotonicClock::subscribe(deadline_, true);
+        engine->queue_async_task(this);
+      } else {
+        TIMERS_MAP->timers_.erase(timer_id_);
+      }
     }
 
     return true;
   }
 
   [[nodiscard]] bool cancel(api::Engine *engine) override {
-    if (!timer_ids_.contains(timer_id_)) {
+    if (!TIMERS_MAP->timers_.contains(timer_id_)) {
       return false;
     }
 
@@ -103,19 +120,16 @@ class TimerTask final : public api::AsyncTask {
   }
 
   static bool clear(int32_t timer_id) {
-    if (!timer_ids_.contains(timer_id)) {
+    if (!TIMERS_MAP->timers_.contains(timer_id)) {
       return false;
     }
 
-    ENGINE->cancel_async_task(timer_ids_[timer_id]);
-    timer_ids_.erase(timer_id);
+    ENGINE->cancel_async_task(TIMERS_MAP->timers_[timer_id]);
+    TIMERS_MAP->timers_.erase(timer_id);
     return true;
   }
 };
 
-std::map<int32_t, PollableHandle> TimerTask::timer_ids_ = {};
-int32_t TimerTask::next_timer_id = 1;
-
 namespace builtins::web::timers {
 
 /**
@@ -193,6 +207,7 @@ constexpr JSFunctionSpec methods[] = {
 
 bool install(api::Engine *engine) {
   ENGINE = engine;
+  TIMERS_MAP.init(engine->cx(), js::MakeUnique<TimersMap>());
   return JS_DefineFunctions(engine->cx(), engine->global(), methods);
 }
 

diff --git a/include/extension-api.h b/include/extension-api.h
@@ -109,7 +109,7 @@ class Engine {
 
   bool has_pending_async_tasks();
   void queue_async_task(AsyncTask *task);
-  bool cancel_async_task(int32_t id);
+  bool cancel_async_task(AsyncTask *task);
 
   void abort(const char *reason);
 

diff --git a/runtime/engine.cpp b/runtime/engine.cpp
@@ -506,6 +506,6 @@ bool api::Engine::debug_logging_enabled() { return ::debug_logging_enabled(); }
 bool api::Engine::has_pending_async_tasks() { return core::EventLoop::has_pending_async_tasks(); }
 
 void api::Engine::queue_async_task(AsyncTask *task) { core::EventLoop::queue_async_task(task); }
-bool api::Engine::cancel_async_task(int32_t id) {
-  return core::EventLoop::cancel_async_task(this, id);
+bool api::Engine::cancel_async_task(AsyncTask *task) {
+  return core::EventLoop::cancel_async_task(this, task);
 }
diff --git a/runtime/event_loop.cpp b/runtime/event_loop.cpp
@@ -28,13 +28,12 @@ void EventLoop::queue_async_task(api::AsyncTask *task) {
   queue.get().tasks.emplace_back(task);
 }
 
-bool EventLoop::cancel_async_task(api::Engine *engine, const int32_t id) {
+bool EventLoop::cancel_async_task(api::Engine *engine, api::AsyncTask *task) {
   const auto tasks = &queue.get().tasks;
   for (auto it = tasks->begin(); it != tasks->end(); ++it) {
-    const auto task = *it;
-    if (task->id() == id) {
-      task->cancel(engine);
+    if (*it == task) {
       tasks->erase(it);
+      task->cancel(engine);
       return true;
     }
   }

diff --git a/runtime/event_loop.h b/runtime/event_loop.h
@@ -46,7 +46,7 @@ class EventLoop {
   /**
    * Remove a queued async task.
    */
-  static bool cancel_async_task(api::Engine *engine, int32_t id);
+  static bool cancel_async_task(api::Engine *engine, api::AsyncTask *task);
 };
 
 } // namespace core