From 5e62ee31d76fcce46d88fea1078552682d082c89 Mon Sep 17 00:00:00 2001
From: Aaron O'Mullan <aaron.omullan@gmail.com>
Date: Tue, 24 May 2022 22:21:32 +0200
Subject: [PATCH] fix(core): op metrics op_names mismatch (#14716)

---
 cli/tests/unit/metrics_test.ts | 13 ++++++++++++-
 core/01_core.js                |  2 +-
 core/bindings.rs               | 32 ++++++++++++++++----------------
 3 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/cli/tests/unit/metrics_test.ts b/cli/tests/unit/metrics_test.ts
index dc103f829b47a1..82ff7ddd06a977 100644
--- a/cli/tests/unit/metrics_test.ts
+++ b/cli/tests/unit/metrics_test.ts
@@ -1,5 +1,5 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
-import { assert } from "./test_util.ts";
+import { assert, assertEquals } from "./test_util.ts";
 
 Deno.test(async function metrics() {
   // Write to stdout to ensure a "data" message gets sent instead of just
@@ -76,3 +76,14 @@ Deno.test(function metricsForOpCrates() {
   assert(m1.opsDispatched > 0);
   assert(m1.opsCompleted > 0);
 });
+
+// Test that op_names == Objects.keys(Deno.core.ops)
+// since building the per-op metrics depends on op_names being complete
+Deno.test(function opNamesMatch() {
+  assertEquals(
+    // @ts-ignore: Deno.core allowed
+    Deno.core.opNames().sort(),
+    // @ts-ignore: Deno.core allowed
+    Object.keys(Deno.core.ops).sort(),
+  );
+});
diff --git a/core/01_core.js b/core/01_core.js
index 2683f4adf90e24..d4842428835f77 100644
--- a/core/01_core.js
+++ b/core/01_core.js
@@ -217,7 +217,7 @@
   function metrics() {
     const [aggregate, perOps] = opSync("op_metrics");
     aggregate.ops = ObjectFromEntries(ArrayPrototypeMap(
-      core.op_names,
+      core.opNames(),
       (opName, opId) => [opName, perOps[opId]],
     ));
     return aggregate;
diff --git a/core/bindings.rs b/core/bindings.rs
index d2335d49cbdc5d..cb57b43fe9bebe 100644
--- a/core/bindings.rs
+++ b/core/bindings.rs
@@ -114,6 +114,9 @@ pub static EXTERNAL_REFERENCES: Lazy<v8::ExternalReferences> =
       v8::ExternalReference {
         function: apply_source_map.map_fn_to(),
       },
+      v8::ExternalReference {
+        function: op_names.map_fn_to(),
+      },
     ])
   });
 
@@ -174,13 +177,10 @@ pub fn initialize_context<'s>(
   // a really weird usecase. Remove this once all
   // tsc ops are static at snapshot time.
   if snapshot_loaded {
-    // Grab the Deno.core & Deno.core.ops objects
-    let core_obj = JsRuntime::grab_global::<v8::Object>(scope, "Deno.core")
-      .expect("Deno.core to exist");
+    // Grab the Deno.core.ops object & init it
     let ops_obj = JsRuntime::grab_global::<v8::Object>(scope, "Deno.core.ops")
       .expect("Deno.core.ops to exist");
     initialize_ops(scope, ops_obj, op_ctxs);
-    initialize_op_names(scope, core_obj, op_ctxs);
     return scope.escape(context);
   }
 
@@ -243,6 +243,7 @@ pub fn initialize_context<'s>(
   set_func(scope, core_val, "destructureError", destructure_error);
   set_func(scope, core_val, "terminate", terminate);
   set_func(scope, core_val, "applySourceMap", apply_source_map);
+  set_func(scope, core_val, "opNames", op_names);
 
   // Direct bindings on `window`.
   set_func(scope, global, "queueMicrotask", queue_microtask);
@@ -250,7 +251,6 @@ pub fn initialize_context<'s>(
   // Bind functions to Deno.core.ops.*
   let ops_obj = JsRuntime::ensure_objs(scope, global, "Deno.core.ops").unwrap();
   initialize_ops(scope, ops_obj, op_ctxs);
-  initialize_op_names(scope, core_val, op_ctxs);
   scope.escape(context)
 }
 
@@ -265,17 +265,6 @@ fn initialize_ops(
   }
 }
 
-fn initialize_op_names(
-  scope: &mut v8::HandleScope,
-  core_obj: v8::Local<v8::Object>,
-  op_ctxs: &[OpCtx],
-) {
-  let names: Vec<&str> = op_ctxs.iter().map(|o| o.decl.name).collect();
-  let k = v8::String::new(scope, "op_names").unwrap().into();
-  let v = serde_v8::to_v8(scope, names).unwrap();
-  core_obj.set(scope, k, v);
-}
-
 pub fn set_func(
   scope: &mut v8::HandleScope<'_>,
   obj: v8::Local<v8::Object>,
@@ -1553,3 +1542,14 @@ fn get_memory_usage(isolate: &mut v8::Isolate) -> MemoryUsage {
     external: s.external_memory(),
   }
 }
+
+fn op_names(
+  scope: &mut v8::HandleScope,
+  _args: v8::FunctionCallbackArguments,
+  mut rv: v8::ReturnValue,
+) {
+  let state_rc = JsRuntime::state(scope);
+  let state = state_rc.borrow();
+  let names: Vec<_> = state.op_ctxs.iter().map(|o| o.decl.name).collect();
+  rv.set(serde_v8::to_v8(scope, names).unwrap());
+}