fix(core): remove unsafe in OpsTracker (denoland#15025)

core/ops.rs

@@ -14,7 +14,6 @@ use futures::task::noop_waker;
 use futures::Future;
 use serde::Serialize;
 use std::cell::RefCell;
-use std::cell::UnsafeCell;
 use std::ops::Deref;
 use std::ops::DerefMut;
 use std::pin::Pin;
@@ -160,9 +159,7 @@ impl OpState {
  resource_table: Default::default(),
  get_error_class_fn: &|_| "Error",
  gotham_state: Default::default(),
- tracker: OpsTracker {
- ops: UnsafeCell::new(vec![Default::default(); ops_count]),
- },
+ tracker: OpsTracker::new(ops_count),
  }
  }
 }

core/ops_metrics.rs

@@ -1,7 +1,8 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
+
 use crate::serde::Serialize;
 use crate::OpId;
-use std::cell::UnsafeCell;
+use std::cell::{RefCell, RefMut};
 
 // TODO(@AaronO): split into AggregateMetrics & PerOpMetrics
 #[derive(Clone, Default, Debug, Serialize)]
@@ -25,18 +26,24 @@ pub struct OpMetrics {
 // TODO(@AaronO): track errors
 #[derive(Default, Debug)]
 pub struct OpsTracker {
- pub ops: UnsafeCell<Vec<OpMetrics>>,
+ ops: RefCell<Vec<OpMetrics>>,
 }
 
 impl OpsTracker {
+ pub fn new(ops_count: usize) -> Self {
+ Self {
+ ops: RefCell::new(vec![Default::default(); ops_count]),
+ }
+ }
+
  pub fn per_op(&self) -> Vec<OpMetrics> {
- self.ops_mut().clone()
+ self.ops.borrow().clone()
  }
 
  pub fn aggregate(&self) -> OpMetrics {
  let mut sum = OpMetrics::default();
 
- for metrics in self.ops_mut().iter() {
+ for metrics in self.ops.borrow().iter() {
  sum.ops_dispatched += metrics.ops_dispatched;
  sum.ops_dispatched_sync += metrics.ops_dispatched_sync;
  sum.ops_dispatched_async += metrics.ops_dispatched_async;
@@ -53,26 +60,14 @@ impl OpsTracker {
  sum
  }
 
- #[allow(clippy::mut_from_ref)]
- #[inline]
- fn ops_mut(&self) -> &mut Vec<OpMetrics> {
- // SAFETY: `OpsTracker` is created after registering ops so it is guaranteed
- // that that `ops` will be initialized.
- unsafe { &mut *self.ops.get() }
- }
-
- #[allow(clippy::mut_from_ref)]
  #[inline]
- fn metrics_mut(&self, id: OpId) -> &mut OpMetrics {
- // SAFETY: `OpsTracker` is created after registering ops, and ops
- // cannot be unregistered during runtime, so it is guaranteed that `id`
- // is not causing out-of-bound access.
- unsafe { self.ops_mut().get_unchecked_mut(id) }
+ fn metrics_mut(&self, id: OpId) -> RefMut<OpMetrics> {
+ RefMut::map(self.ops.borrow_mut(), |ops| &mut ops[id])
  }
 
  #[inline]
  pub fn track_sync(&self, id: OpId) {
- let metrics = self.metrics_mut(id);
+ let mut metrics = self.metrics_mut(id);
  metrics.ops_dispatched += 1;
  metrics.ops_completed += 1;
  metrics.ops_dispatched_sync += 1;
@@ -81,14 +76,14 @@ impl OpsTracker {
 
  #[inline]
  pub fn track_async(&self, id: OpId) {
- let metrics = self.metrics_mut(id);
+ let mut metrics = self.metrics_mut(id);
  metrics.ops_dispatched += 1;
  metrics.ops_dispatched_async += 1;
  }
 
  #[inline]
  pub fn track_async_completed(&self, id: OpId) {
- let metrics = self.metrics_mut(id);
+ let mut metrics = self.metrics_mut(id);
  metrics.ops_completed += 1;
  metrics.ops_completed_async += 1;
  }

ops/lib.rs

@@ -293,7 +293,7 @@ fn codegen_v8_sync(
 
  let result = Self::call::<#type_params>(#args_head #args_tail);
 
- let op_state = &mut ctx.state.borrow();
+ let op_state = &*ctx.state.borrow();
  op_state.tracker.track_sync(ctx.id);
 
  #ret