Skip to content

Commit

Permalink
fix(turbopack-ecmascript-runtime): prevent hanging when top level awa…
Browse files Browse the repository at this point in the history
…it is skipped (vercel/turborepo#8409)

### Description

A "race condition" was causing the queue to never be resolved.

### Testing Instructions

Added a new test for it


Fixes #65278
  • Loading branch information
ForsakenHarmony authored Jun 12, 2024
1 parent f60a99a commit d22c892
Show file tree
Hide file tree
Showing 65 changed files with 457 additions and 41 deletions.
32 changes: 20 additions & 12 deletions crates/turbopack-ecmascript-runtime/js/src/shared/runtime-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,12 +333,20 @@ const turbopackQueues = Symbol("turbopack queues");
const turbopackExports = Symbol("turbopack exports");
const turbopackError = Symbol("turbopack error");

const enum QueueStatus {
Unknown = -1,
Unresolved = 0,
Resolved = 1,
}

type AsyncQueueFn = (() => void) & { queueCount: number };
type AsyncQueue = AsyncQueueFn[] & { resolved: boolean };
type AsyncQueue = AsyncQueueFn[] & {
status: QueueStatus;
};

function resolveQueue(queue?: AsyncQueue) {
if (queue && !queue.resolved) {
queue.resolved = true;
if (queue && queue.status !== QueueStatus.Resolved) {
queue.status = QueueStatus.Resolved;
queue.forEach((fn) => fn.queueCount--);
queue.forEach((fn) => (fn.queueCount-- ? fn.queueCount++ : fn()));
}
Expand All @@ -355,11 +363,13 @@ type AsyncModuleExt = {
type AsyncModulePromise<T = Exports> = Promise<T> & AsyncModuleExt;

function wrapDeps(deps: Dep[]): AsyncModuleExt[] {
return deps.map((dep) => {
return deps.map((dep): AsyncModuleExt => {
if (dep !== null && typeof dep === "object") {
if (isAsyncModuleExt(dep)) return dep;
if (isPromise(dep)) {
const queue: AsyncQueue = Object.assign([], { resolved: false });
const queue: AsyncQueue = Object.assign([], {
status: QueueStatus.Unresolved,
});

const obj: AsyncModuleExt = {
[turbopackExports]: {},
Expand All @@ -381,12 +391,10 @@ function wrapDeps(deps: Dep[]): AsyncModuleExt[] {
}
}

const ret: AsyncModuleExt = {
return {
[turbopackExports]: dep,
[turbopackQueues]: () => {},
};

return ret;
});
}

Expand All @@ -401,7 +409,7 @@ function asyncModule(
hasAwait: boolean
) {
const queue: AsyncQueue | undefined = hasAwait
? Object.assign([], { resolved: true })
? Object.assign([], { status: QueueStatus.Unknown })
: undefined;

const depQueues: Set<AsyncQueue> = new Set();
Expand Down Expand Up @@ -450,7 +458,7 @@ function asyncModule(
function fnQueue(q: AsyncQueue) {
if (q !== queue && !depQueues.has(q)) {
depQueues.add(q);
if (q && !q.resolved) {
if (q && q.status === QueueStatus.Unresolved) {
fn.queueCount++;
q.push(fn);
}
Expand All @@ -474,8 +482,8 @@ function asyncModule(

body(handleAsyncDependencies, asyncResult);

if (queue) {
queue.resolved = false;
if (queue && queue.status === QueueStatus.Unknown) {
queue.status = QueueStatus.Unresolved;
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/turbopack-tests/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as expectMod from "expect";
import * as jest from "jest-circus";
import * as expectMod from "./tests/execution/node_modules/expect";
import * as jest from "./tests/execution/node_modules/jest-circus";

export {};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
it("should not hang", async () => {
debugger;
const { test } = await import("./wrapper");

expect(test()).toBe(5);
}, 1000);
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const x = 5;

const a = 10;
if (a !== 10) {
// intentionally nothing, the skipped await point causes the problem
await 0;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { x } from "./repro.js";

export function test() {
return x;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const magicAsyncModule = require("./magic")
const magicAsyncModule = require("./magic");

describe("complex wasm", () => {
it("should be possible to use imported memory", async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import { fibonacci } from "./fibonacci.wasm";
export { add, factorial, fibonacci };

export function factorialJavascript(i) {
if (i < 1) return 1;
return i * factorialJavascript(i - 1);
if (i < 1) return 1;
return i * factorialJavascript(i - 1);
}

export function fibonacciJavascript(i) {
if (i < 2) return 1;
return fibonacciJavascript(i - 1) + fibonacciJavascript(i - 2);
if (i < 2) return 1;
return fibonacciJavascript(i - 1) + fibonacciJavascript(i - 2);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from "./unknown.js";

await 1;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
it("should handle re-export from async modules correctly", async () => {
await import("./test.js");
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from "./async-unknown.js";
export { a } from "./async-unknown.js";
export default "default";
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import * as ns from "./reexport-async-unknown.js?ns";
import { a, b, c } from "./reexport-async-unknown.js?named";
import value from "./reexport-async-unknown.js?default";

function nsObj(m) {
Object.defineProperty(m, Symbol.toStringTag, { value: "Module" });
return m;
}

expect(ns).toEqual(
nsObj({
default: "default",
a: "a",
b: "b",
c: "c",
})
);

expect(a).toBe("a");
expect(b).toBe("b");
expect(c).toBe("c");

expect(value).toBe("default");
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const o = {
a: "a",
b: "b",
c: "c",
};

module.exports = Object(o);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import x from "./shared";

export default x + " world";
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import x from "./shared";

export default x + " world";
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
it("should allow to import an async module twice", async () => {
const result = await require("./main");
expect(result.default).toBe("hello world, hello world");
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import a from "./a";
import b from "./b";

export default a + ", " + b;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
await 1;
await 1;
export default "hello";
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import i, { foo } from "./won't-run-tla";

it("should have value imported from won't-run-tla", async () => {
expect(i).toBe(42);
expect(foo).toBe(undefined);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
global.someNonExistentVariable && (await "test");
const foo = global.otherSomeNonExistentVariable && (await 43);
export default 42;
export { foo };
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./async";

report("a");
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { report } from "../tick";

report("async before");
await 0;
report("async middle");
await 0;
report("async after");
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { report } from "../tick";
import "./d";

report("async2 before");
await 0;
report("async2 middle");
await 0;
report("async2 after");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./async";

report("b");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./b";

report("c");
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { report } from "../tick";
import "./c";
import "./a";

report("d");
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { report } from "../tick";

report("e");
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { report } from "../tick";
import "./e";
import "./async2";

report("f");
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { report } from "../tick";
import "./async";
import "./b";

report("a");
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { report } from "../tick";

report("async before");
await 0;
report("async middle");
await 0;
report("async after");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./c";

report("b");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./async";

report("c");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./c";

report("d");
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { report } from "../tick";
import "./a";
import "./d";

report("async before");
await 0;
report("async middle");
await 0;
report("async after");
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { report } from "../tick";
import "./b";
import "./a";

report("a before");
await 0;
report("a after");
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { report } from "../tick";

report("b");
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { report } from "../tick";
import "./b";

report("a before");
await 0;
report("a after");
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { report } from "../tick";
import "./c";

report("b before");
await 0;
report("b after");
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { report } from "../tick";
import "./a";

report("c before");
await 0;
report("c after");
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { report } from "../tick";
import "./x";
import "./y";

report("index");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./a";

report("x");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./b";

report("y");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./async";

report("a");
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { report } from "../tick";

report("async before");
await 0;
report("async middle");
await 0;
report("async after");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./async";

report("b");
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { report } from "../tick";
import "./a";
import "./b";
import "./x";

report("index");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { report } from "../tick";
import "./a";

report("x");
Loading

0 comments on commit d22c892

Please sign in to comment.