Repository: denoland/deno_core (rust)

Issue:
Error without any stack trace
I often encounter the following error in my program, but I don't know where the error is coming from.

<img width="901" alt="截屏2023-11-08 下午6 52 22" src="https://github.com/denoland/deno_core/assets/15227926/ade21821-f58a-4c1d-b8bb-5e98ae440e7f">

based on this error message, there seems to be some bug in 01_core.js:

<img width="429" alt="截屏2023-11-08 下午7 05 03" src="https://github.com/denoland/deno_core/assets/15227926/70f7c5ae-53f5-4f47-bfe7-4af86146164b">

`getPromise(promiseId)` could potentially return undefined, but this situation is not being handled here.


deno version: 
```
deno 1.37.2 (release, x86_64-unknown-linux-gnu)
v8 11.8.172.13
typescript 5.2.2
```




Pull request description:
fix(core): don't fail on 32-bit rollover of promise IDs
Fixes #313

There's a theoretical problem if we stomp over an existing promise in the promise map but this gets us to a better place.

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index fafa18f..27d2795 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -28,6 +28,10 @@ jobs:
       - name: Install Rust
         uses: dsherret/rust-toolchain-file@v1
 
+      - name: Install Rust (nightly)
+        if: contains(matrix.os, 'ubuntu')
+        uses: dtolnay/rust-toolchain@nightly
+
       - name: Install Deno
         uses: denoland/setup-deno@v1
 
@@ -52,6 +56,14 @@ jobs:
           cargo clippy --locked --release --all-features --all-targets -- -D clippy::all
           deno lint
 
+      - name: Lint deps
+        if: contains(matrix.os, 'ubuntu')
+        run: |
+          cargo install cargo-machete --locked
+          cargo machete
+          cargo install cargo-udeps --locked
+          cargo +nightly udeps
+
       - name: Cargo test
         run: cargo test --locked --release --all-features --bins --tests --examples && cargo test --doc
 
diff --git a/Cargo.lock b/Cargo.lock
index fb547d3..26e8923 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -268,10 +268,8 @@ dependencies = [
  "deno_ops",
  "deno_unsync",
  "futures",
- "indexmap 2.1.0",
  "libc",
  "log",
- "once_cell",
  "parking_lot",
  "pin-project",
  "pretty_assertions",
@@ -300,16 +298,11 @@ dependencies = [
 name = "deno_ops"
 version = "0.104.0"
 dependencies = [
- "lazy-regex",
- "once_cell",
- "pmutil",
  "pretty_assertions",
  "prettyplease",
- "proc-macro-crate",
  "proc-macro-rules",
  "proc-macro2",
  "quote",
- "regex",
  "strum",
  "strum_macros",
  "syn 2.0.39",
@@ -541,9 +534,9 @@ dependencies = [
 
 [[package]]
 name = "getrandom"
-version = "0.2.10"
+version = "0.2.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "be4136b2a15dd319360be1c07d9933517ccf0be8f16bf62a3bee4f0d618df427"
+checksum = "fe9006bed769170c11f845cf00c7c1e9092aeb3f268e007c3e760ac68008070f"
 dependencies = [
  "cfg-if",
  "libc",
@@ -650,29 +643,6 @@ version = "1.0.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "af150ab688ff2122fcef229be89cb50dd66af9e01a4ff320cc137eecc9bacc38"
 
-[[package]]
-name = "lazy-regex"
-version = "3.0.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e723bd417b2df60a0f6a2b6825f297ea04b245d4ba52b5a22cb679bdf58b05fa"
-dependencies = [
- "lazy-regex-proc_macros",
- "once_cell",
- "regex",
-]
-
-[[package]]
-name = "lazy-regex-proc_macros"
-version = "3.0.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0f0a1d9139f0ee2e862e08a9c5d0ba0470f2aa21cd1e1aa1b1562f83116c725f"
-dependencies = [
- "proc-macro2",
- "quote",
- "regex",
- "syn 2.0.39",
-]
-
 [[package]]
 name = "lazy_static"
 version = "1.4.0"
@@ -687,9 +657,9 @@ checksum = "89d92a4743f9a61002fae18374ed11e7973f530cb3a3255fb354818118b2203c"
 
 [[package]]
 name = "linux-raw-sys"
-version = "0.4.10"
+version = "0.4.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "da2479e8c062e40bf0066ffa0bc823de0a9368974af99c9f6df941d2c231e03f"
+checksum = "969488b55f8ac402214f3f5fd243ebb7206cf82de60d3172994707a4bcc2b829"
 
 [[package]]
 name = "lock_api"
@@ -950,15 +920,6 @@ dependencies = [
  "syn 2.0.39",
 ]
 
-[[package]]
-name = "proc-macro-crate"
-version = "2.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7e8366a6159044a37876a2b9817124296703c586a5c92e2c53751fa06d8d43e8"
-dependencies = [
- "toml_edit",
-]
-
 [[package]]
 name = "proc-macro-hack"
 version = "0.5.20+deprecated"
@@ -1881,23 +1842,6 @@ dependencies = [
  "syn 2.0.39",
 ]
 
-[[package]]
-name = "toml_datetime"
-version = "0.6.5"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3550f4e9685620ac18a50ed434eb3aec30db8ba93b0287467bca5826ea25baf1"
-
-[[package]]
-name = "toml_edit"
-version = "0.20.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "70f427fce4d84c72b5b732388bf4a9f4531b53f74e2887e3ecb2481f68f66d81"
-dependencies = [
- "indexmap 2.1.0",
- "toml_datetime",
- "winnow",
-]
-
 [[package]]
 name = "tracing"
 version = "0.1.40"
@@ -2150,15 +2094,6 @@ version = "0.48.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538"
 
-[[package]]
-name = "winnow"
-version = "0.5.19"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "829846f3e3db426d4cee4510841b71a8e58aa2a76b1132579487ae430ccd9c7b"
-dependencies = [
- "memchr",
-]
-
 [[package]]
 name = "yansi"
 version = "0.5.1"
diff --git a/Cargo.toml b/Cargo.toml
index 5fa7a63..8fe5074 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -36,13 +36,10 @@ indexmap = "2"
 libc = "0.2.126"
 log = "0.4.17"
 num-bigint = { version = "0.4", features = ["rand"] }
-once_cell = "1.17.1"
 parking_lot = "0.12.0"
 pin-project = "1"
 pretty_assertions = "1.3.0"
 rand = "0.8.5"
-regex = "1"
-lazy-regex = "3"
 prettyplease = "0.2.15"
 serde = { version = "1", features = ["derive"] }
 serde_bytes = "0.11"
@@ -65,8 +62,6 @@ url = { version = "2", features = ["serde", "expose_internals"] }
 proc-macro2 = "1"
 quote = "1"
 syn = { version = "2", features = ["full", "extra-traits"] }
-pmutil = "0.6.1"
-proc-macro-crate = "2.0.0"
 proc-macro-rules = "0.4.0"
 
 # NB: the `bench` and `release` profiles must remain EXACTLY the same.
diff --git a/core/01_core.js b/core/01_core.js
index 49bd561..d0f6762 100644
--- a/core/01_core.js
+++ b/core/01_core.js
@@ -71,7 +71,7 @@
   registerErrorClass("TypeError", TypeError);
   registerErrorClass("URIError", URIError);
 
-  let nextPromiseId = 1;
+  let nextPromiseId = 0xffffffff;
   const promiseMap = new SafeMap();
   const RING_SIZE = 4 * 1024;
   const NO_PROMISE = null; // Alias to null is faster than plain nulls
@@ -119,12 +119,18 @@
     const outOfBounds = promiseId < nextPromiseId - RING_SIZE;
     if (outOfBounds) {
       const promise = MapPrototypeGet(promiseMap, promiseId);
+      if (!promise) {
+        throw "Missing promise in map @ " + promiseId;
+      }
       MapPrototypeDelete(promiseMap, promiseId);
       return promise;
     }
     // Otherwise take from ring
     const idx = promiseId % RING_SIZE;
     const promise = promiseRing[idx];
+    if (!promise) {
+      throw "Missing promise in ring @ " + promiseId;
+    }
     promiseRing[idx] = NO_PROMISE;
     return promise;
   }
@@ -292,7 +298,7 @@ for (let i = 0; i < 10; i++) {
   s += `
       case ${i}:
         fn = function async_op_${i}(${args}) {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id, ${args});
             if (maybeResult !== undefined) {
@@ -327,7 +333,7 @@ for (let i = 0; i < 10; i++) {
     switch (originalOp.length - 1) {
       case 0:
         fn = function async_op_0() {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id);
             if (maybeResult !== undefined) {
@@ -351,7 +357,7 @@ for (let i = 0; i < 10; i++) {
 
       case 1:
         fn = function async_op_1(a) {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id, a);
             if (maybeResult !== undefined) {
@@ -375,7 +381,7 @@ for (let i = 0; i < 10; i++) {
 
       case 2:
         fn = function async_op_2(a, b) {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id, a, b);
             if (maybeResult !== undefined) {
@@ -399,7 +405,7 @@ for (let i = 0; i < 10; i++) {
 
       case 3:
         fn = function async_op_3(a, b, c) {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id, a, b, c);
             if (maybeResult !== undefined) {
@@ -423,7 +429,7 @@ for (let i = 0; i < 10; i++) {
 
       case 4:
         fn = function async_op_4(a, b, c, d) {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id, a, b, c, d);
             if (maybeResult !== undefined) {
@@ -447,7 +453,7 @@ for (let i = 0; i < 10; i++) {
 
       case 5:
         fn = function async_op_5(a, b, c, d, e) {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id, a, b, c, d, e);
             if (maybeResult !== undefined) {
@@ -471,7 +477,7 @@ for (let i = 0; i < 10; i++) {
 
       case 6:
         fn = function async_op_6(a, b, c, d, e, f) {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id, a, b, c, d, e, f);
             if (maybeResult !== undefined) {
@@ -495,7 +501,7 @@ for (let i = 0; i < 10; i++) {
 
       case 7:
         fn = function async_op_7(a, b, c, d, e, f, g) {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id, a, b, c, d, e, f, g);
             if (maybeResult !== undefined) {
@@ -519,7 +525,7 @@ for (let i = 0; i < 10; i++) {
 
       case 8:
         fn = function async_op_8(a, b, c, d, e, f, g, h) {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id, a, b, c, d, e, f, g, h);
             if (maybeResult !== undefined) {
@@ -543,7 +549,7 @@ for (let i = 0; i < 10; i++) {
 
       case 9:
         fn = function async_op_9(a, b, c, d, e, f, g, h, i) {
-          const id = nextPromiseId++;
+          const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
           try {
             const maybeResult = originalOp(id, a, b, c, d, e, f, g, h, i);
             if (maybeResult !== undefined) {
@@ -581,7 +587,7 @@ for (let i = 0; i < 10; i++) {
   }
 
   function opAsync(name, ...args) {
-    const id = nextPromiseId++;
+    const id = (nextPromiseId = (nextPromiseId + 1) & 0xffffffff);
     try {
       const maybeResult = asyncOps[name](id, ...new SafeArrayIterator(args));
       if (maybeResult !== undefined) {
@@ -632,6 +638,14 @@ for (let i = 0; i < 10; i++) {
     ops.op_unref_op(promiseId);
   }
 
+  function refOpPromise(promise) {
+    refOp(promise[promiseIdSymbol]);
+  }
+
+  function unrefOpPromise(promise) {
+    unrefOp(promise[promiseIdSymbol]);
+  }
+
   function resources() {
     return ObjectFromEntries(ops.op_resources());
   }
@@ -839,6 +853,8 @@ for (let i = 0; i < 10; i++) {
     opCallTraces,
     refOp,
     unrefOp,
+    refOpPromise,
+    unrefOpPromise,
     setReportExceptionCallback,
     setPromiseHooks,
     close,
diff --git a/core/Cargo.toml b/core/Cargo.toml
index 1f7f53d..74ac0b1 100644
--- a/core/Cargo.toml
+++ b/core/Cargo.toml
@@ -25,10 +25,8 @@ bytes.workspace = true
 deno_ops.workspace = true
 deno_unsync.workspace = true
 futures.workspace = true
-indexmap.workspace = true
 libc.workspace = true
 log.workspace = true
-once_cell.workspace = true
 parking_lot.workspace = true
 pin-project.workspace = true
 serde.workspace = true
diff --git a/core/benches/ops/async.rs b/core/benches/ops/async.rs
index da75239..ad4c418 100644
--- a/core/benches/ops/async.rs
+++ b/core/benches/ops/async.rs
@@ -9,6 +9,8 @@ deno_core::extension!(
   ops = [
     op_void,
     op_make_external,
+    op_call_promise_resolver,
+    op_resolve_promise,
     op_async_void,
     op_async_void_lazy,
     op_async_void_lazy_nofast,
@@ -23,6 +25,22 @@ deno_core::extension!(
   ],
 );
 
+#[op2]
+pub fn op_call_promise_resolver(scope: &mut v8::HandleScope, f: &v8::Function) {
+  let recv = v8::undefined(scope).try_into().unwrap();
+  f.call(scope, recv, &[]);
+}
+
+#[op2]
+pub fn op_resolve_promise<'s>(
+  scope: &'s mut v8::HandleScope,
+) -> v8::Local<'s, v8::Promise> {
+  let resolver = v8::PromiseResolver::new(scope).unwrap();
+  let value = v8::undefined(scope).try_into().unwrap();
+  resolver.resolve(scope, value).unwrap();
+  resolver.get_promise(scope)
+}
+
 #[op2(fast)]
 pub fn op_void() {}
 
@@ -158,6 +176,28 @@ fn baseline_promise(b: &mut Bencher) {
   );
 }
 
+/// Tests the overhead of execute_script with a promise.
+fn baseline_op_promise_resolver(b: &mut Bencher) {
+  bench_op(
+    b,
+    BENCH_COUNT,
+    "op_call_promise_resolver",
+    1,
+    "{ let { promise, resolve } = Promise.withResolvers(); op_call_promise_resolver(resolve); await promise; }",
+  );
+}
+
+/// Tests the overhead of execute_script with a promise.
+fn baseline_op_promise(b: &mut Bencher) {
+  bench_op(
+    b,
+    BENCH_COUNT,
+    "op_resolve_promise",
+    0,
+    "await op_resolve_promise();",
+  );
+}
+
 /// Tests the overhead of execute_script, but also returns a value so we can make sure things are
 /// working.
 fn bench_op_async_void_deferred_return(b: &mut Bencher) {
@@ -206,6 +246,8 @@ benchmark_group!(
   benches,
   baseline,
   baseline_promise,
+  baseline_op_promise_resolver,
+  baseline_op_promise,
   baseline_sync,
   bench_op_async_yield,
   bench_op_async_yield_lazy,
diff --git a/core/external.rs b/core/external.rs
index d27ba92..3d84140 100644
--- a/core/external.rs
+++ b/core/external.rs
@@ -27,8 +27,6 @@ macro_rules! external {
   };
 }
 
-pub use external;
-
 pub trait Externalizable {
   fn external_marker() -> usize;
   fn external_name() -> &'static str;
diff --git a/core/lib.deno_core.d.ts b/core/lib.deno_core.d.ts
index fc78658..6510d13 100644
--- a/core/lib.deno_core.d.ts
+++ b/core/lib.deno_core.d.ts
@@ -21,6 +21,14 @@ declare namespace Deno {
      * if there are only "unref" promises left. */
     function unrefOp(promiseId: number): void;
 
+    /** Mark following promise as "ref", ie. event loop won't exit
+     * until all "ref" promises are resolved. All async ops are "ref" by default. */
+    function refOpPromise<T>(promise: Promise<T>): void;
+
+    /** Mark following promise as "unref", ie. event loop will exit
+     * if there are only "unref" promises left. */
+    function unrefOpPromise<T>(promise: Promise<T>): void;
+
     /**
      * List of all registered ops, in the form of a map that maps op
      * name to function.
diff --git a/ops/Cargo.toml b/ops/Cargo.toml
index 39c72c2..1546591 100644
--- a/ops/Cargo.toml
+++ b/ops/Cargo.toml
@@ -15,14 +15,9 @@ path = "./lib.rs"
 proc-macro = true
 
 [dependencies]
-lazy-regex.workspace = true
-once_cell.workspace = true
-pmutil.workspace = true
-proc-macro-crate.workspace = true
 proc-macro-rules.workspace = true
 proc-macro2.workspace = true
 quote.workspace = true
-regex.workspace = true
 strum.workspace = true
 strum_macros.workspace = true
 syn.workspace = true
diff --git a/serde_v8/Cargo.toml b/serde_v8/Cargo.toml
index ccc33d8..47fe6f6 100644
--- a/serde_v8/Cargo.toml
+++ b/serde_v8/Cargo.toml
@@ -18,7 +18,6 @@ bytes.workspace = true
 derive_more.workspace = true
 num-bigint.workspace = true
 serde.workspace = true
-serde_bytes.workspace = true
 smallvec = { workspace = true, features = ["union"] }
 thiserror.workspace = true
 v8.workspace = true
@@ -26,6 +25,7 @@ v8.workspace = true
 [dev-dependencies]
 bencher.workspace = true
 serde_json.workspace = true
+serde_bytes.workspace = true
 
 [[example]]
 name = "basic"


Test patch:
diff --git a/core/runtime/tests/jsrealm.rs b/core/runtime/tests/jsrealm.rs
index 886a50f..479ae29 100644
--- a/core/runtime/tests/jsrealm.rs
+++ b/core/runtime/tests/jsrealm.rs
@@ -459,8 +459,7 @@ async fn js_realm_ref_unref_ops() {
         runtime.v8_isolate(),
         "",
         r#"
-          let promiseIdSymbol = Symbol.for("Deno.core.internalPromiseId");
-          Deno.core.unrefOp(promise[promiseIdSymbol]);
+          Deno.core.unrefOpPromise(promise);
         "#,
       )
       .unwrap();
@@ -471,8 +470,7 @@ async fn js_realm_ref_unref_ops() {
         runtime.v8_isolate(),
         "",
         r#"
-          let promiseIdSymbol = Symbol.for("Deno.core.internalPromiseId");
-          Deno.core.unrefOp(promise[promiseIdSymbol]);
+          Deno.core.unrefOpPromise(promise);
         "#,
       )
       .unwrap();