refactor(core): validate promise id in refOp (denoland#13905)

fanbaby · Mar 11, 2022 · b198bfd · b198bfd
1 parent f9b4d26
commit b198bfd
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 20 deletions.
diff --git a/cli/tests/unit/ref_unref_test.ts b/cli/tests/unit/ref_unref_test.ts
@@ -0,0 +1,10 @@
+import { assertNotEquals, execCode } from "./test_util.ts";
+
+Deno.test("[unrefOp] unref'ing invalid ops does not have effects", async () => {
+ const [statusCode, _] = await execCode(`
+ Deno.core.unrefOp(-1);
+ setTimeout(() => { throw new Error() }, 10)
+ `);
+ // Invalid unrefOp call doesn't affect exit condition of event loop
+ assertNotEquals(statusCode, 0);
+});
diff --git a/cli/tests/unit/test_util.ts b/cli/tests/unit/test_util.ts
@@ -27,3 +27,21 @@ export function pathToAbsoluteFileUrl(path: string): URL {
 
  return new URL(`file:https://${Deno.build.os === "windows" ? "/" : ""}${path}`);
 }
+
+const decoder = new TextDecoder();
+
+export async function execCode(code: string) {
+ const p = Deno.run({
+ cmd: [
+ Deno.execPath(),
+ "eval",
+ "--unstable",
+ "--no-check",
+ code,
+ ],
+ stdout: "piped",
+ });
+ const [status, output] = await Promise.all([p.status(), p.output()]);
+ p.close();
+ return [status.code, decoder.decode(output)];
+}
diff --git a/cli/tests/unit/timers_test.ts b/cli/tests/unit/timers_test.ts
@@ -6,11 +6,10 @@ import {
  Deferred,
  deferred,
  delay,
+ execCode,
  unreachable,
 } from "./test_util.ts";
 
-const decoder = new TextDecoder();
-
 Deno.test(async function functionParameterBindingSuccess() {
  const promise = deferred();
  let count = 0;
@@ -578,21 +577,6 @@ Deno.test(
  },
 );
 
-async function execCode(code: string) {
- const p = Deno.run({
- cmd: [
- Deno.execPath(),
- "eval",
- "--unstable",
- code,
- ],
- stdout: "piped",
- });
- const [status, output] = await Promise.all([p.status(), p.output()]);
- p.close();
- return [status.code, decoder.decode(output)];
-}
-
 Deno.test({
  name: "unrefTimer",
  permissions: { run: true },

diff --git a/core/01_core.js b/core/01_core.js
@@ -29,7 +29,7 @@
  } = window.__bootstrap.primordials;
 
  // Available on start due to bindings.
- const { opcallSync, opcallAsync } = window.Deno.core;
+ const { opcallSync, opcallAsync, refOp_, unrefOp_ } = window.Deno.core;
 
  let opsCache = {};
  const errorMap = {};
@@ -99,6 +99,17 @@
  return promise;
  }
 
+ function hasPromise(promiseId) {
+ // Check if out of ring bounds, fallback to map
+ const outOfBounds = promiseId < nextPromiseId - RING_SIZE;
+ if (outOfBounds) {
+ return MapPrototypeHas(promiseMap, promiseId);
+ }
+ // Otherwise check it in ring
+ const idx = promiseId % RING_SIZE;
+ return promiseRing[idx] != NO_PROMISE;
+ }
+
  function ops() {
  return opsCache;
  }
@@ -172,6 +183,20 @@
  return unwrapOpResult(opcallSync(opsCache[opName], arg1, arg2));
  }
 
+ function refOp(promiseId) {
+ if (!hasPromise(promiseId)) {
+ return;
+ }
+ refOp_(promiseId);
+ }
+
+ function unrefOp(promiseId) {
+ if (!hasPromise(promiseId)) {
+ return;
+ }
+ unrefOp_(promiseId);
+ }
+
  function resources() {
  return ObjectFromEntries(opSync("op_resources"));
  }
@@ -252,6 +277,8 @@
  enableOpCallTracing,
  isOpCallTracingEnabled,
  opCallTraces,
+ refOp,
+ unrefOp,
  });
 
  ObjectAssign(globalThis.__bootstrap, { core });

diff --git a/core/bindings.rs b/core/bindings.rs
@@ -173,8 +173,8 @@ pub fn initialize_context<'s>(
  // Bind functions to Deno.core.*
  set_func(scope, core_val, "opcallSync", opcall_sync);
  set_func(scope, core_val, "opcallAsync", opcall_async);
- set_func(scope, core_val, "refOp", ref_op);
- set_func(scope, core_val, "unrefOp", unref_op);
+ set_func(scope, core_val, "refOp_", ref_op);
+ set_func(scope, core_val, "unrefOp_", unref_op);
  set_func(
  scope,
  core_val,