Skip to content

chore: add contribution guide#2

Merged
underfin merged 1 commit into
mainfrom
add-contribution-guide
Sep 19, 2023
Merged

chore: add contribution guide#2
underfin merged 1 commit into
mainfrom
add-contribution-guide

Conversation

@underfin

Copy link
Copy Markdown
Contributor

No description provided.

@underfin underfin merged commit d746c77 into main Sep 19, 2023
@underfin underfin deleted the add-contribution-guide branch September 19, 2023 07:49
graphite-app Bot pushed a commit that referenced this pull request Jun 24, 2026
Related to #9884 and #9733

## TL;DR

`@rolldown/browser` ≥ 1.1.2 throws `RuntimeError: Atomics.wait cannot be called in this context` when bundling **on the browser main thread**, as soon as a *second* build runs in the same session. 1.1.1 was fine. The cause is the `defer_drop` optimization added in 1.1.2 (`crates/rolldown/src/utils/defer_drop.rs`): `BundleFactory::build_bundle` synchronously calls `defer_drop::drain()`, which does `Condvar::wait` — i.e. it **parks the calling thread** — to wait for the *previous* build's deferred drop to finish. On wasm that lowers to `memory.atomic.wait`, which is illegal on the browser main thread. Fix: on wasm, drop inline (don't defer, don't drain).

## Symptom

In a cross-origin-isolated page, calling `rolldown()` / `generate()` **on the page main thread** (not in a Worker):

```
RuntimeError: Atomics.wait cannot be called in this context
    at wasm-function[...] (rolldown-binding.wasm32-wasi.wasm)
    ...
```

- First build of the session succeeds; the **second and later** builds throw.
- The rolldown REPL hits this every time, because it runs **two builds per "Bundle"**: it first compiles `rolldown.config.ts` (build #1), then bundles the entry (build #2).
- Running the exact same code **inside a Web Worker** does not throw (parking is legal off the main thread).

## Background: why the main thread cannot park

When one thread must wait for another (e.g. "wait until the background free is done"), it **parks** — sleeps until notified. The primitive is a futex; in wasm it is the `memory.atomic.wait` instruction (JS: `Atomics.wait`). Browsers **forbid `memory.atomic.wait` on the main thread** (a sleeping main thread would freeze the event loop / UI), so the engine throws `Atomics.wait cannot be called in this context`. Worker threads may park freely. `std::sync::{Mutex, Condvar}` on `wasm32-wasip1-threads` lower to this instruction, so any `Condvar::wait` reached on the browser main thread crashes.

## Root cause

`defer_drop` (new in 1.1.2) moves the ~15 ms free of the link-stage output off the hot thread onto a rayon worker:

- `bundle.rs` — after `generate()`: `defer_drop::spawn_drop(link_stage_output)` → `PENDING += 1; rayon::spawn(move || drop(value))`.
- `bundle_factory.rs` — `BundleFactory::build_bundle` (a **synchronous** function that runs on whatever thread called rolldown): `defer_drop::drain()`, which `Condvar::wait`s while `PENDING > 0`, to guarantee a build never overlaps the previous build's frees.

On native this is fine (the caller is a tokio/rayon worker that may block). On the browser **main thread** it is not: `build_bundle` of the **second** build parks the main thread waiting for the **first** build's still-running background drop.

```mermaid
sequenceDiagram
    autonumber
    participant M as Browser main thread
    participant W as rayon worker (web worker)

    Note over M: Build #1 — e.g. compile rolldown.config.ts
    M->>M: generate() returns
    M->>W: spawn_drop(link_stage_output) — PENDING = 1
    activate W
    Note over W: freeing ~15 ms in the background

    Note over M: Build #2 — bundle the entry (same synchronous turn, no yield)
    M->>M: BundleFactory::build_bundle()
    M->>M: drain() sees PENDING > 0
    M->>M: Condvar::wait()  ➜  wasm memory.atomic.wait
    Note over M: blocking the browser main thread is forbidden
    M-->>M: ❌ throw "Atomics.wait cannot be called in this context"
    deactivate W
```

Between build #1's `spawn_drop` and build #2's `drain()` the main thread never yields, so the background worker has not retired the drop yet → `PENDING` is still `> 0` → `drain()` parks → crash. (`getModuleInfo`/plugin hooks are unrelated — they run on the bundling worker; the crash reproduces with no plugins at all, purely from doing two builds.)

## Why 1.1.1 was fine, and why it is not a dependency regression

- **1.1.1** has no `defer_drop`: it frees the link-stage output inline on the hot thread. No background drop, no `drain`, no parking on the caller → the main thread never blocks.
- Dependency bumps were ruled out by diffing the exact versions used by 1.1.1 vs 1.1.2: `napi` `3.9.1→3.9.3` leaves `tokio_runtime.rs` byte-identical (only a TSFN monomorphization refactor + a stream rewrite); `@tybys/wasm-util`/emnapi `1.11.0→1.11.1` only changes JS (`ensureBufferFor`), the wasm-side thread/wait C code is byte-identical; `@napi-rs/cli` `3.7.1→3.7.2` emits a byte-identical wasi loader/worker template. The only new code on the "synchronous-`Condvar::wait`-on-the-calling-thread" path is `defer_drop.rs` itself.

## The fix

On wasm, drop inline (the deferral's ~15 ms saving is irrelevant for interactive browser builds, and there is no spare blockable thread the main thread is allowed to wait on):

```diff
+#[cfg(not(target_family = "wasm"))]
 use std::sync::{Condvar, Mutex, PoisonError};

+#[cfg(not(target_family = "wasm"))]
 static PENDING: Mutex<usize> = Mutex::new(0);
+#[cfg(not(target_family = "wasm"))]
 static PENDING_IS_ZERO: Condvar = Condvar::new();

+#[cfg(not(target_family = "wasm"))]
 struct PendingGuard;
+#[cfg(not(target_family = "wasm"))]
 impl Drop for PendingGuard { /* unchanged */ }

 pub fn spawn_drop<T: Send + 'static>(value: T) {
-  *PENDING.lock().unwrap_or_else(PoisonError::into_inner) += 1;
-  rayon::spawn(move || {
-    let _guard = PendingGuard;
-    drop(value);
-  });
+  // On wasm the thread that later calls `drain()` may be the browser main
+  // thread, where the matching `Condvar::wait` lowers to `memory.atomic.wait`
+  // and is illegal. Drop inline so there is never a cross-build wait.
+  #[cfg(target_family = "wasm")]
+  drop(value);
+  #[cfg(not(target_family = "wasm"))]
+  {
+    *PENDING.lock().unwrap_or_else(PoisonError::into_inner) += 1;
+    rayon::spawn(move || {
+      let _guard = PendingGuard;
+      drop(value);
+    });
+  }
 }

 pub fn drain() {
-  let mut pending = PENDING.lock().unwrap_or_else(PoisonError::into_inner);
-  while *pending > 0 {
-    pending = PENDING_IS_ZERO.wait(pending).unwrap_or_else(PoisonError::into_inner);
-  }
+  // wasm drops inline in `spawn_drop`, so nothing is ever pending; a
+  // `Condvar::wait` here would crash on the browser main thread.
+  #[cfg(not(target_family = "wasm"))]
+  {
+    let mut pending = PENDING.lock().unwrap_or_else(PoisonError::into_inner);
+    while *pending > 0 {
+      pending = PENDING_IS_ZERO.wait(pending).unwrap_or_else(PoisonError::into_inner);
+    }
+  }
 }
```

### Trade-off / alternative

This disables the deferral on **all** wasm, including wasi-threads runs inside a Worker where parking would be legal. That is the simplest correct option and costs only the ~15 ms inline-free on wasm. If the optimization is worth keeping for the worker case, the alternative is to keep `spawn_drop` deferring on wasm but make `drain()` **not block on the browser main thread** — either skip the wait there (the pending drop simply finishes in the background; the count stays bounded by "one per build") or detect the main thread (e.g. via emnapi's `_emnapi_is_main_browser_thread`). The inline approach avoids that runtime/FFI coupling.

## Reproduction & verification

In a cross-origin-isolated (COOP/COEP) page, run two builds on the main thread:

```js
const core = await import('.../@rolldown/browser/dist/index.browser.mjs')
const binding = await import('.../@rolldown/browser/dist/rolldown-binding.wasi-browser.js')
binding.__volume.fromJSON({ '/index.ts': 'export const foo = 42' })
for (let i = 0; i < 2; i++) {
  const b = await core.rolldown({ input: ['/index.ts'], cwd: '/' })
  await b.generate({ format: 'esm' })
}
```

- **Before:** the 2nd `generate()` throws `Atomics.wait cannot be called in this context`.
- **After:** both builds succeed.

The same code inside a Web Worker already succeeds with or without this change (parking is legal off the main thread), so the patch only affects the browser-main-thread path. (Builds run in a Worker are unaffected; native is unchanged.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant