Repository: denoland/deno_core
Pull request #483 (2024-01-25 01:48:09)

feat(core): sanitization for timers


Patch:
diff --git a/core/runtime/stats.rs b/core/runtime/stats.rs
index 4b4e834..a36d8aa 100644
--- a/core/runtime/stats.rs
+++ b/core/runtime/stats.rs
@@ -27,10 +27,17 @@ impl RuntimeActivityStatsFactory {
         .resources
         .push((resource.0, resource.1.to_string()))
     }
-    let timers = TimerStats {
-      intervals: vec![],
-      timers: vec![],
+    let timer_count = self.context_state.timers.len();
+    let mut timers = TimerStats {
+      timers: Vec::with_capacity(timer_count),
+      repeats: BitSet::with_capacity(timer_count),
     };
+    for (timer_id, repeats) in &self.context_state.timers.iter() {
+      if repeats {
+        timers.repeats.insert(timers.timers.len());
+      }
+      timers.timers.push(timer_id as usize);
+    }
     RuntimeActivityStats {
       context_state: self.context_state.clone(),
       op: self.context_state.pending_ops.stats(),
@@ -45,10 +52,10 @@ pub struct ResourceOpenStats {
 }
 
 pub struct TimerStats {
-  #[allow(dead_code)] // coming soon
   pub(super) timers: Vec<usize>,
-  #[allow(dead_code)] // coming soon
-  pub(super) intervals: Vec<usize>,
+  /// `repeats` is a bitset that reports whether a given index in the ID array
+  /// is an interval (if true) or a timer (if false).
+  pub(super) repeats: BitSet,
 }
 
 /// Information about in-flight ops, open resources, active timers and other runtime-specific
@@ -65,16 +72,19 @@ pub struct RuntimeActivityStats {
 pub enum RuntimeActivity {
   AsyncOp(PromiseId, String),
   Resource(ResourceId, String),
-  Timer(usize, usize),
-  Interval(usize, usize),
+  Timer(usize),
+  Interval(usize),
 }
 
 impl RuntimeActivityStats {
   /// Capture the data within this [`RuntimeActivityStats`] as a [`RuntimeActivitySnapshot`]
   /// with details of activity.
   pub fn dump(&self) -> RuntimeActivitySnapshot {
-    let mut v =
-      Vec::with_capacity(self.op.ops.len() + self.resources.resources.len());
+    let mut v = Vec::with_capacity(
+      self.op.ops.len()
+        + self.resources.resources.len()
+        + self.timers.timers.len(),
+    );
     let ops = self.context_state.op_ctxs.borrow();
     for op in self.op.ops.iter() {
       v.push(RuntimeActivity::AsyncOp(
@@ -85,6 +95,13 @@ impl RuntimeActivityStats {
     for resource in self.resources.resources.iter() {
       v.push(RuntimeActivity::Resource(resource.0, resource.1.clone()))
     }
+    for i in 0..self.timers.timers.len() {
+      if self.timers.repeats.contains(i) {
+        v.push(RuntimeActivity::Interval(self.timers.timers[i]));
+      } else {
+        v.push(RuntimeActivity::Timer(self.timers.timers[i]));
+      }
+    }
     RuntimeActivitySnapshot { active: v }
   }
 
@@ -137,6 +154,35 @@ impl RuntimeActivityStats {
       }
     }
 
+    let mut a = BitSet::new();
+    for timer in after.timers.timers.iter() {
+      a.insert(*timer);
+    }
+    for index in 0..before.timers.timers.len() {
+      let timer = before.timers.timers[index];
+      if a.remove(timer) {
+        // continuing op
+      } else {
+        // before, but not after
+        if before.timers.repeats.contains(index) {
+          disappeared.push(RuntimeActivity::Interval(timer));
+        } else {
+          disappeared.push(RuntimeActivity::Timer(timer));
+        }
+      }
+    }
+    for index in 0..after.timers.timers.len() {
+      let timer = after.timers.timers[index];
+      if a.contains(timer) {
+        // after but not before
+        if after.timers.repeats.contains(index) {
+          appeared.push(RuntimeActivity::Interval(timer));
+        } else {
+          appeared.push(RuntimeActivity::Timer(timer));
+        }
+      }
+    }
+
     RuntimeActivityDiff {
       appeared,
       disappeared,
diff --git a/core/web_timeout.rs b/core/web_timeout.rs
index 3460432..11c0361 100644
--- a/core/web_timeout.rs
+++ b/core/web_timeout.rs
@@ -5,8 +5,10 @@ use cooked_waker::Wake;
 use cooked_waker::WakeRef;
 use futures::Future;
 use std::cell::Cell;
+use std::cell::Ref;
 use std::cell::RefCell;
 use std::cell::UnsafeCell;
+use std::collections::btree_set;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::mem::MaybeUninit;
@@ -89,6 +91,42 @@ impl<T> Default for WebTimers<T> {
   }
 }
 
+pub(crate) struct WebTimersIterator<'a, T> {
+  data: Ref<'a, BTreeMap<WebTimerId, (T, bool)>>,
+  timers: Ref<'a, BTreeSet<TimerKey>>,
+}
+
+impl<'a, T> IntoIterator for &'a WebTimersIterator<'a, T> {
+  type IntoIter = WebTimersIteratorImpl<'a, T>;
+  type Item = (u64, bool);
+
+  fn into_iter(self) -> Self::IntoIter {
+    WebTimersIteratorImpl {
+      data: &self.data,
+      timers: self.timers.iter(),
+    }
+  }
+}
+
+pub(crate) struct WebTimersIteratorImpl<'a, T> {
+  data: &'a BTreeMap<WebTimerId, (T, bool)>,
+  timers: btree_set::Iter<'a, TimerKey>,
+}
+
+impl<'a, T> Iterator for WebTimersIteratorImpl<'a, T> {
+  type Item = (u64, bool);
+  fn next(&mut self) -> Option<Self::Item> {
+    loop {
+      let Some(item) = self.timers.next() else {
+        return None;
+      };
+      if self.data.contains_key(&item.1) {
+        return Some((item.1, !matches!(item.2, TimerType::Once)));
+      }
+    }
+  }
+}
+
 struct MutableSleep {
   sleep: UnsafeCell<Option<Sleep>>,
   ready: Cell<bool>,
@@ -210,6 +248,15 @@ unsafe impl ViaRawPointer for MutableSleepWaker {
 }
 
 impl<T: Clone> WebTimers<T> {
+  /// Returns an internal iterator that locks the internal data structures for the period
+  /// of iteration. Calling other methods on this collection will cause a panic.
+  pub(crate) fn iter(&self) -> WebTimersIterator<T> {
+    WebTimersIterator {
+      data: self.data_map.borrow(),
+      timers: self.timers.borrow(),
+    }
+  }
+
   /// Refs a timer by ID. Invalid IDs are ignored.
   pub fn ref_timer(&self, id: WebTimerId) {
     if let Some((_, unrefd)) = self.data_map.borrow_mut().get_mut(&id) {


Test patch:
diff --git a/testing/checkin/runtime/async.ts b/testing/checkin/runtime/async.ts
index d075394..026fa96 100644
--- a/testing/checkin/runtime/async.ts
+++ b/testing/checkin/runtime/async.ts
@@ -98,6 +98,11 @@ export class StatsCollection {
     return this.countResourceActivity("Resource");
   }
 
+  countTimers(): number {
+    return this.countResourceActivity("Timer") +
+      this.countResourceActivity("Interval");
+  }
+
   get empty(): boolean {
     return this.data.length == 0;
   }
diff --git a/testing/unit/stats_test.ts b/testing/unit/stats_test.ts
index 4da84a4..a9d78fc 100644
--- a/testing/unit/stats_test.ts
+++ b/testing/unit/stats_test.ts
@@ -40,3 +40,21 @@ test(function testStatsResources() {
   const diff = StatsFactory.diff(statsBefore, statsAfter);
   assert(diff.empty);
 });
+
+test(function testTimers() {
+  using statsBefore = StatsFactory.capture();
+
+  const timeout = setTimeout(() => null, 1000);
+  const interval = setInterval(() => null, 1000);
+
+  using statsMiddle = StatsFactory.capture();
+  const diffMiddle = StatsFactory.diff(statsBefore, statsMiddle);
+  assertEquals(0, diffMiddle.disappeared.countTimers());
+  assertEquals(2, diffMiddle.appeared.countTimers());
+  clearTimeout(timeout);
+  clearInterval(interval);
+
+  using statsAfter = StatsFactory.capture();
+  const diff = StatsFactory.diff(statsBefore, statsAfter);
+  assert(diff.empty);
+});