diff --git a/CMakeLists.txt b/CMakeLists.txt index 1577c299..659d1fca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,7 +30,11 @@ if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) include(FetchContent) if (WIN32) - SET(COMPILE_FLAGS "/GR- /W0") + # This build doesn't go through Mozilla's moz.build system, which + # normally defines XP_WIN on Windows -- without it, SpiderMonkey headers + # that branch on it (e.g. PlatformMutex.h) fall through to a POSIX path + # that doesn't exist here. + SET(COMPILE_FLAGS "/GR- /W0 /DXP_WIN") SET(OPTIMIZED "/O2") SET(UNOPTIMIZED "/Od") diff --git a/SPIDERMONKEY_VERSION_BUMP.md b/SPIDERMONKEY_VERSION_BUMP.md new file mode 100644 index 00000000..bfb84a1b --- /dev/null +++ b/SPIDERMONKEY_VERSION_BUMP.md @@ -0,0 +1,783 @@ +# pythonmonkey: moving off the nightly-alpha SpiderMonkey build — handover notes + +**Status: BUILD SUCCEEDS, CORE FUNCTIONALITY VERIFIED.** This document tracks +every change made while rebuilding pythonmonkey against a current +mozilla-central snapshot instead of the ~19-month-old nightly alpha it +previously shipped (`mozjs-136a1.dll`). Written for handover to the +pythonmonkey/dcp team — read this before trusting or shipping the resulting +build. **See the Testing section near the end for exactly what was and +wasn't verified before you rely on this.** + +New engine: `mozjs-157a1.dll`, built from mozilla-central commit +`1704651e7d6c706fcb753adab577e0954d61cee0` (current trunk as of this work, +2026-09-14) — see the "Which commit to target" section below for why this +specific commit and not a numbered Firefox release. + +--- + +## Why + +A security review of what `pip install dcp` actually puts on a machine found +that pythonmonkey embeds `mozjs-136a1.dll` — not a small standalone library, +but a build of SpiderMonkey (Firefox's JS engine) out of a full +`firefox-source` checkout. The `136a1` is Mozilla's own suffix for **Nightly +Alpha 1**, an unstable, pre-release development snapshot, explicitly "not for +general users." That build predates Firefox 136's eventual stable release — +including an emergency out-of-band patch (136.0.4, shipped 2025-03-27) for a +sandbox-escape vulnerability that was being **actively exploited in the wild** +(CVE-2025-2857), and the broader memory-safety fixes in that release cycle +(MFSA 2025-14). + +Checked `pythonmonkey`'s own upstream `main` branch (`git fetch origin`, +compared `mozcentral.version`): it is **also** still pinned to the exact same +`6bca861985ba51920c1cacc21986af01c51bd690` alpha commit as this local clone, +unchanged since at least their last commit. This is not a stale-local-clone +problem — it's what pythonmonkey ships to everyone today. + +## What target was chosen, and why + +This GitHub mirror (`mozilla-firefox/firefox`, what `setup.sh` actually +downloads from) only tracks mozilla-central **trunk** — it has no per-release +tags, only a rolling `last-mozilla-central` tag. There is no way to pin an +exact "Firefox 136.0.4" commit from this specific source. The practical +equivalent is a trunk commit safely after the point those fixes landed (Mozilla +lands security fixes in trunk before or alongside backporting them to release +branches). + +Initially picked a conservative target (~3.5 weeks after 136.0.4's ship date, +April 2025) to minimize source drift from the currently-working alpha. On +reflection (correctly challenged mid-task): that doesn't actually solve the +underlying problem — an 18-months-old-by-now snapshot would *also* read as +"stale, unpatched" to a future reviewer. Since this mirror only ever offers a +trunk snapshot regardless of which commit is picked, there's no "stable +channel" to fall back to either way — so the right choice is the **freshest** +trunk commit, not a stale one. + +**Final target: `1704651e7d6c706fcb753adab577e0954d61cee0`**, dated +2026-09-06 (~1 week before this work started, deliberately not the literal +tip-of-trunk at the moment of picking, to sidestep any short-lived transient +build breakage mozilla-central occasionally has). + +Old pin preserved at `mozcentral.version.orig-backup-136a1` for rollback. + +--- + +## Changes made, in order + +### 1. `mozcentral.version` + +```diff +- 6bca861985ba51920c1cacc21986af01c51bd690 ++ 1704651e7d6c706fcb753adab577e0954d61cee0 +``` + +### 2. `setup.sh` — several fixes, all confirmed necessary by real build failures (not speculative) + +**a. Rust install step made idempotent.** Was unconditional on every run. +Re-running `rustup-init.sh` when Rust is already installed downloads a fresh +installer exe and executes it, which on this Windows machine gets blocked by +Windows Defender ("Permission denied" — see finding 3 below for the pattern). +Skips the whole block if the 1.85 toolchain is already present, matching the +existing pattern right below it (the Poetry-install skip): + +```diff ++ if command -v rustup >/dev/null && rustup toolchain list 2>/dev/null | grep -q '^1\.85'; then ++ echo "Rust 1.85 toolchain already installed, skipping rustup-init" ++ else + echo "Installing rust compiler" + ... + curl ... | sh -s -- -y ... --default-toolchain 1.85 ++ fi + CARGO_BIN="$HOME/.cargo/bin/cargo" +- $CARGO_BIN install cbindgen ++ command -v cbindgen >/dev/null || $CARGO_BIN install cbindgen +``` + +**b. `wget` replaced with `curl` for the Firefox source download.** +`wget.exe` (confirmed both the MSYS2 copy and — implicitly — any copy) is +blocked outright by a **Windows Defender Application Control (WDAC) policy** +on this machine: `An Application Control policy has blocked this file`, +confirmed directly via a native PowerShell invocation, not a PATH/permissions +issue. This is new since the original 136a1 build session — nothing in that +session's own log mentions any AppLocker/WDAC block. **This machine's security +posture has tightened since the last build**, plausibly related to DWAN-prep +work happening in parallel — worth flagging to whoever manages this machine's +policy. `curl` (both Windows' own and MSYS2's) is unaffected; `unzip` is also +unaffected. Swapped just the `wget` call: + +```diff +- wget -c -q -O firefox-source-${MOZCENTRAL_VERSION}.zip https://... ++ curl -fsSL -o firefox-source-${MOZCENTRAL_VERSION}.zip https://... +``` + +**c. Removed `--disable-explicit-resource-management` configure flag.** +Worked around Bugzilla 1940342 (a header/lib enum mismatch from when the +`using` JS syntax was newly landing in nightly, circa early 2025). On the new +snapshot this is now an *unrecognized* configure option (`InvalidOptionError: +Unknown option`) — the feature has evidently shipped/stabilized since, taking +the flag (and presumably the underlying bug) with it. Removed rather than +guessing a replacement. + +**d. `MOZILLABUILD` `KeyError` fix reapplied.** This is the *same* fix already +made once before, ad-hoc, during the earlier one-line SharedArrayBuffer/Atomics +build session against the old `136a1` engine (not part of this PR) — but that +fix was applied directly to a file *inside* the ephemeral `firefox-source` +checkout, not to this persistent `setup.sh`, so it was lost when +`firefox-source` was deleted and re-fetched for the new commit. Re-applied, +and this time added as a proper `sed` patch in `setup.sh` itself (matching the +existing pattern of the other ~10 patches) so it survives future re-extracts: + +```diff ++ sed -i'' -e 's/os\.environ\["MOZILLABUILD"\]/os.environ.get("MOZILLABUILD", "")/g' ./python/mozbuild/mozbuild/backend/visualstudio.py # LOCAL PATCH: ... +``` + +**e. Poetry install made idempotent, and kept — not dropped.** An earlier +version of this patch skipped installing Poetry altogether, reasoning that it +was only consumed later in this same script's `.git/hooks/pre-commit` +dev-tooling branch and thus "irrelevant to actually building +SpiderMonkey/pythonmonkey." That reasoning was wrong — it broke that branch's +`$POETRY_BIN run pip install autopep8` line for anyone whose clone does take +it, by deleting `POETRY_BIN`'s own definition along with the install step. +Caught during review/retesting, not by a build failure. Fixed the *actual* +problem instead (this machine has no `python3` on `PATH`, only `python`, so +the real installer's `python3 - --version ...` invocation failed outright) +and made the install idempotent, matching the Rust fix in (a): + +```diff ++ if command -v "$POETRY_BIN" >/dev/null || [ -x "$POETRY_BIN" ]; then ++ echo "Poetry already installed, skipping" ++ else + echo "Installing poetry" +- curl -sSL https://install.python-poetry.org | python3 - --version "1.7.1" ++ PYTHON_FOR_POETRY=$(command -v python3 || command -v python) ++ curl -sSL https://install.python-poetry.org | "$PYTHON_FOR_POETRY" - --version "1.7.1" + ... ++ "$POETRY_BIN" self add 'poetry-dynamic-versioning[plugin]' ++ fi +``` + +### 3. Rust toolchain override: 1.85 → `stable` (1.98.1) + +The new mozilla-central snapshot's own `configure` now hard-requires +`rustc >= 1.90.0` (`ERROR: Rust compiler 1.85.1 is too old`) — pythonmonkey's +own 1.85 pin is unrelated to this; it's Mozilla's minimum that moved. Rust's +stable channel (1.98.1) was already installed on this machine from earlier +exploration. Switched pythonmonkey-src's directory-level `rustup override` +from `1.85-x86_64-pc-windows-msvc` to `stable-x86_64-pc-windows-msvc`, rather +than installing yet another specific pinned version. This relies on Rust's +strong backward-compatibility guarantee (stable code essentially never breaks +on a newer compiler) — **not independently verified against pythonmonkey's +own Rust code specifically**, just against the fact that the build proceeded +past this point without new Rust-level errors. + +``` +rustup override set stable # (run inside pythonmonkey-src) +``` + +The 1.85 toolchain itself was left installed (not removed) — no reason to. + +### 4. `CMakeLists.txt` — `XP_WIN` now defined globally for the Windows build + +```diff + if (WIN32) +- SET(COMPILE_FLAGS "/GR- /W0") ++ SET(COMPILE_FLAGS "/GR- /W0 /DXP_WIN") +``` + +pythonmonkey's own `.cc` files (and the SpiderMonkey public headers they pull +in) are compiled directly by this CMake/clang-cl build, not through Mozilla's +own `moz.build` system — which normally defines `XP_WIN` (Mozilla's standard +"building for Windows" macro) for every object file it compiles itself. +Without it, any SpiderMonkey header that branches on `defined(XP_WIN)` +(assuming, reasonably, that Mozilla's own build always defines it on Windows) +silently takes its POSIX/pthread branch instead — confirmed via a real build +failure: `mozilla/PlatformMutex.h` trying to `#include ` (doesn't +exist for an MSVC/clang-cl target), which cascaded into missing-type errors in +`mozilla/UniquePtrExtensions.h`. One instance of this exact class of bug was +already patched, per-file, in the original build (`BaseProfilerUtils.h`, +`defined(XP_WIN)` → `defined(_WIN32)`, still present as a `setup.sh` sed +patch) — since the newer Mozilla snapshot has apparently grown *more* files +with this pattern, fixing it globally here is more robust than continuing to +patch individual headers as they're discovered. + +### 5. `src/BufferType.cc` — SpiderMonkey internal API adaptation — **NEEDS TEAM REVIEW** + +`JS_GetArrayBufferViewFixedData` no longer exists in the new SpiderMonkey — +not renamed, redesigned. This is the **first non-mechanical fix** in this +whole effort: a real SpiderMonkey-internal-C++-API break requiring judgment +about GC/memory safety, not just an environment/build-tooling issue. + +```diff + bool isSharedMemory; + if (!JS_GetArrayBufferViewBuffer(cx, typedArray, &isSharedMemory)) return nullptr; + +- uint8_t __destBuf[0] = {}; +- uint8_t *data = JS_GetArrayBufferViewFixedData(typedArray, __destBuf, 0); +- if (data == nullptr) { // shared memory or still having inline data ++ if (isSharedMemory) { + PyErr_SetString(PyExc_TypeError, "PythonMonkey cannot coerce TypedArrays backed by shared memory."); + return nullptr; + } ++ ++ JS::AutoAssertNoGC nogc(cx); // see below re: why AutoAssertNoGC, not the base AutoRequireNoGC ++ bool isSharedMemory2; ++ uint8_t *data = static_cast(JS_GetArrayBufferViewData(typedArray, &isSharedMemory2, nogc)); ++ if (data == nullptr) { ++ PyErr_SetString(PyExc_TypeError, "PythonMonkey cannot coerce TypedArrays backed by shared memory."); ++ return nullptr; ++ } +``` + +**The reasoning, and exactly what hasn't been verified:** + +The old function's safety contract was: return `nullptr` if the TypedArray's +data is still stored inline (i.e. GC-movable, because it lives inside the +TypedArray object shell rather than a separately-allocated ArrayBuffer). The +new function (`JS_GetArrayBufferViewData`) drops that runtime check entirely +and instead requires a `JS::AutoRequireNoGC` token from the caller. + +**`AutoRequireNoGC` (`js/GCAPI.h`) itself has protected constructor/destructor +— it's a base marker type, not directly instantiable** (confirmed by a real +build error when first tried). Used `JS::AutoAssertNoGC` instead, a public +subclass that — better than a pure marker — actually performs a runtime +assertion in diagnostic builds that no GC occurs while it's alive (a no-op in +release builds, same as the base class would have been). This gives genuine +runtime verification of the safety property in debug/diagnostic builds, not +just a compile-time formality. Even so, this change does **not** mechanically +preserve the *original* function's safety guarantee (which rejected movable +data outright rather than asserting on it) — it relies on reasoning as well +as this assertion: + +The existing `JS_GetArrayBufferViewBuffer()` call, immediately before this, +already exists specifically (per its own original comment, unchanged) to force +any inline/movable TypedArray data to be promoted to a real, stably-allocated +ArrayBuffer first. If that reasoning is correct, the pointer +`JS_GetArrayBufferViewData` returns immediately afterward should already be +backed by stable (non-inline) storage — meaning the specific hazard the old +function's runtime check guarded against should already be closed off before +this new call ever runs, and the `AutoRequireNoGC` token is satisfiable +truthfully rather than just suppressing a compiler complaint. + +**This has NOT been independently verified against SpiderMonkey's actual GC +behavior.** pythonmonkey hands this pointer to Python as a `Py_buffer`, which +Python code can hold onto indefinitely — well past the scope of the local +`nogc` guard. If the reasoning above is wrong in some edge case (e.g. a +TypedArray configuration where `JS_GetArrayBufferViewBuffer`'s promotion +doesn't fully eliminate movability), this would be a real, silent +use-after-free / data-corruption bug, worse than the build simply failing. + +**Recommended before trusting this build for anything beyond +experimentation**: stress-test the TypedArray-to-Python-buffer path +specifically under a compacting/moving GC configuration +(`--enable-gczeal` or equivalent SpiderMonkey debug-build GC-stress mode), +and/or get this specific diff reviewed by someone with real SpiderMonkey GC +internals expertise. Do not ship this change based on this document's +reasoning alone. + +### 6. `include/JobQueue.hh` / `src/JobQueue.cc` — SpiderMonkey API addition, mechanical fix (low risk) + +`JS::JobQueue::getHostDefinedData` (the base class pythonmonkey's `JobQueue` +overrides) gained a second out-parameter, `incumbentGlobal` — previously that +concept was only supplied as an *input* to the separate `enqueuePromiseJob` +method, which pythonmonkey's implementation already ignores entirely (no +incumbent-global tracking at all; jobs are just forwarded to Python's asyncio +event loop). Unlike the `BufferType.cc` fix, **this one is not a judgment +call**: pythonmonkey's existing `getHostDefinedData` already took the "we +don't need this" stance for the original `data` out-param +(`data.set(nullptr); return true;`), so the new `incumbentGlobal` param gets +exactly the same treatment, consistent with the file's own established +pattern rather than inventing new behavior: + +```diff +- bool JobQueue::getHostDefinedData(JSContext *cx, JS::MutableHandle data) const { ++ bool JobQueue::getHostDefinedData(JSContext *cx, JS::MutableHandle incumbentGlobal, JS::MutableHandle data) const { ++ incumbentGlobal.set(nullptr); // We don't need the incumbent global + data.set(nullptr); // We don't need the host defined data + return true; + } +``` + +(Header declaration in `JobQueue.hh` updated to match.) + +--- + +### 7. `include/JobQueue.hh` / `src/JobQueue.cc` / `src/modules/pythonmonkey/pythonmonkey.cc` — SpiderMonkey JobQueue redesign (architecture change — NEEDS REVIEW) + +This is qualitatively different from every fix above it: not a renamed +parameter or a missing macro, but a real redesign of how SpiderMonkey expects +an embedder to receive promise/microtask jobs. Flagged to the team explicitly +before proceeding; the decision (from the project owner) was to keep going and +document it thoroughly rather than stop here. + +**What changed, and how this was confirmed (not guessed):** the build failed +with `error: only virtual member functions can be marked 'override'` on +pythonmonkey's `enqueuePromiseJob` and `empty()` overrides. Reading the new +`JS::JobQueue` base class in full (`js/public/Promise.h`) confirmed both +methods are gone from the interface entirely — not renamed, removed. Two new +pure-virtual methods were added instead: `getHostDefinedGlobal` and (already +present, unrelated) `saveJobQueue`. To understand *why*, and find the +replacement mechanism, traced every call site of `jobQueue->` across all of +`js/src` (7 total, none enqueue-shaped), read Gecko's own real embedding +(`xpcom/base/CycleCollectedJSContext.h/.cpp`) and SpiderMonkey's own reference +embedding (`js/src/vm/JSContext.h`'s `InternalJobQueue`), and finally read +`js/public/friend/MicroTask.h`, which turned out to document the whole new +design inline (see its `[SMDOC]` comment block). + +**The new design, in short:** SpiderMonkey no longer calls out to the embedder +for every job as it's created. Instead it queues jobs itself, internally +(`cx->microTaskQueues`, via `EnqueueJob()` in `js/src/builtin/Promise.cpp`). +The embedder is expected to *pull* jobs from that queue itself, inside its +`JobQueue::runJobs()` override, whenever it wants a "microtask checkpoint" to +happen. That pull is triggered by the embedder calling the free function +`js::RunJobs(cx)` (declared in `jsfriendapi.h`) — note this is **not** the +same thing as the `runJobs()` *method* pythonmonkey overrides, despite the +identical name: `js::RunJobs(cx)` is the public entry point, and its entire +body is `cx->jobQueue->runJobs(cx)` — i.e. it's what *calls* our override. + +Previously, pythonmonkey never needed to call `js::RunJobs(cx)` anywhere, +because `enqueuePromiseJob` forwarded each job to Python's asyncio event loop +the instant SpiderMonkey created it — there was no engine-side queue to drain. +Under the new design, if nothing ever calls `js::RunJobs(cx)`, jobs pile up in +`cx->microTaskQueues` forever and **no promise ever resolves**. So this fix +has two parts: + +**(a) `JobQueue::runJobs()` now does real work** (`src/JobQueue.cc`), instead +of being a no-op. It loops while `JS::HasAnyMicroTasks(cx)`, dequeues each job +via `JS::DequeueNextMicroTask` + `JS::ToMaybeWrappedJSMicroTask`, and forwards +it to the Python event loop — recreating what `enqueuePromiseJob` used to do +per-job, just pull-based now instead of push-based: + +```diff +- void JobQueue::runJobs(JSContext *cx) { +- // Do nothing +- } ++ void JobQueue::runJobs(JSContext *cx) { ++ while (JS::HasAnyMicroTasks(cx)) { ++ JS::RootedValue entry(cx, JS::DequeueNextMicroTask(cx)); ++ if (entry.isNull()) break; ++ JS::Rooted job(cx, JS::ToMaybeWrappedJSMicroTask(entry)); ++ if (!job) continue; ++ auto *rootedJob = new JS::PersistentRooted(cx, job); ++ // ... pack (cx, rootedJob) into a PyCFunction closure, enqueue it on ++ // the running Python event-loop (see runMicroTaskCallback), same as ++ // enqueuePromiseJob's loop.enqueue(callback) did before. ++ } ++ } +``` + +One real difference from the old `enqueuePromiseJob`: `job` there was a +`JS::HandleObject` documented as an ECMA-262 Job (i.e. a plain callable +function with no arguments), which pythonmonkey converted straight to a +Python callable via `pyTypeFactory(cx, jobv)` and handed to Python. The new +`JS::JSMicroTask*` is **not** a generically-callable function — it's an opaque +engine-internal representation that must be executed specifically via +`JS::RunJSMicroTask(cx, job)`, inside `AutoRealm`d to +`JS::GetExecutionGlobalFromJSMicroTask(job)` (this exact usage pattern is +documented in the `[SMDOC]` block at the top of `js/public/friend/MicroTask.h` +— not improvised). So instead of reusing `pyTypeFactory` to wrap `job` +itself, a new small native PyCFunction (`runMicroTaskCallback`) was added, +modelled directly on the existing `dispatchToEventLoop`/`callDispatchFunc` +pattern already in this same file (which smuggles a `(JSContext*, +JS::Dispatchable*)` pair through a Python closure the same way) — packs +`(cx, rootedJob)` as a 2-tuple, and when Python's loop finally calls it, calls +`JS::RunJSMicroTask` and reports failure via the existing +`setSpiderMonkeyException(cx)` helper (same one used throughout +`pythonmonkey.cc`). + +`JS::JSMicroTask` is a type alias for plain `JSObject` (confirmed directly in +`MicroTask.h`: `using JSMicroTask = JSObject;`), so it can be kept alive +across the gap between "dequeued here" and "Python's event loop calls back, +possibly much later" the same way pythonmonkey already keeps +FinalizationRegistry callbacks alive elsewhere in this file: a heap-allocated +`JS::PersistentRooted`, freed once the callback actually runs. + +**(b) `pythonmonkey.cc` now calls `js::RunJobs(GLOBAL_CX)`** once, immediately +after every top-level `JS_ExecuteScript()` call — the natural equivalent of +the HTML spec's "clean up after running script" microtask checkpoint, and (as +far as could be found) the only place in pythonmonkey's own source that a +checkpoint like this was ever implicitly happening before (via the old +immediate-forwarding design). + +**`getHostDefinedGlobal`** (the other new pure-virtual method) was given the +same "we don't track this" stance pythonmonkey already takes for +`getHostDefinedData`'s params — `out.set(nullptr); return true;` — which +matches SpiderMonkey's own reference embedding +(`InternalJobQueue::getHostDefinedGlobal` in `js/src/vm/JSContext.cpp`) +exactly, so this part is low-risk / pattern-consistent rather than a guess. + +**NEEDS REVIEW — this is the least-verified change in this entire document, +more so than the `BufferType.cc` GC fix:** +- Whether draining exactly once per `JS_ExecuteScript()` call is the *right* + cadence for this embedding (vs., say, needing a checkpoint after every + re-entry into JS, or after every Python-side `await` of a JS promise) has + not been verified against real async/await interop test cases — only + against "does it compile and does the basic shape make sense." +- GC-safety of holding a `JS::PersistentRooted` across an + arbitrary, unbounded real-world delay (Python's event loop may not run the + callback for a while) is modelled on the pre-existing, working + `finalizationRegistryCallbacks` pattern in this same file, but has not been + independently confirmed for `JSMicroTask` objects specifically. +- The ordering/interleaving semantics (does a JS promise chain still resolve + in the same relative order it used to, now that jobs are batch-pulled per + checkpoint instead of pushed one at a time?) has not been tested. +- **Before trusting this for anything beyond experimentation**: write and run + a test that chains multiple `await`s across the Python/JS boundary + (`pm.eval` returning a Promise that resolves another Promise, etc.) and + confirms both completion and ordering, not just successful compilation. + +**UPDATE — this was tested, and the concern above was real.** Once the build +first succeeded (fix #10 below), a smoke test awaiting even a single, +already-resolved JS Promise from Python hung indefinitely. Root cause: the +one `js::RunJobs(GLOBAL_CX)` call added above (after `JS_ExecuteScript`) +only checkpoints the *first* batch of jobs created during top-level script +execution. It does not cover the other two places jobs get freshly enqueued +into `cx->microTaskQueues`, both entirely outside of any `JS_ExecuteScript` +call: + +1. **`PromiseType::getPyObject`** (`src/PromiseType.cc`) — called when Python + code `await`s a JS Promise. `JS::AddPromiseReactions` attaches a reaction + callback; if the promise is already settled (the common case for a + same-tick resolution), this immediately enqueues a job that nothing was + draining. +2. **`futureOnDoneCallback`** (`src/PromiseType.cc`) — called from a Python + `asyncio.Future`'s done-callback (i.e. from Python's event loop, not from + JS at all) to resolve/reject a JS Promise that JS was awaiting on a Python + awaitable. `JS::ResolvePromise`/`JS::RejectPromise` here can trigger that + promise's own already-attached reactions, again with nothing draining them. +3. **`runMicroTaskCallback`** (`src/JobQueue.cc`, part of this same fix #7) — + running one microtask (e.g. one `await` in a chain) can enqueue the next + one; the callback returned without re-checkpointing, so a promise chain + with more than one `await` stalled after the first hop even once (1) and + (2) were fixed. + +Fixed by adding `js::RunJobs(cx)` at all three points — mechanical once the +pattern was identified (same call used above), but finding *where* it was +missing required actually running async code, not just getting a clean +compile. **This is the concrete confirmation that "compiles" and "works" are +different claims for this whole JobQueue rewrite** — treat any other +not-yet-exercised code path in this rewrite (the debug queue, +`saveJobQueue`/`SavedJobQueue` used by the Debugger API, `isDrainingStopped`) +with the same suspicion until it's actually been run. + +Retested after this fix: a single `await` of an already-resolved Promise, a +two-hop `await` chain inside an async function (verifying both completion +*and* ordering), and a `setTimeout`-based Promise (exercising the unrelated, +pre-existing `PyEventLoop::enqueueWithDelay` timer path) — see the Testing +section near the end of this document for exact results. + +### 8. `src/JobQueue.cc` — `mozilla::Unused` / `mozilla/Unused.h` removed upstream, mechanical fix (low risk) + +Next build error after fix #7: `fatal error: 'mozilla/Unused.h' file not found`. +Confirmed this isn't a path/environment issue — the header (and the +`mozilla::Unused` helper it declared) is genuinely gone from the current +mozilla-central snapshot's `mfbt/` directory, not just moved (checked: absent +from `mfbt/`, and grepping `dom/`, `xpcom/base/`, `js/src/vm/` for +`mozilla::Unused` turns up zero uses anywhere in current upstream code, +confirming it's been fully purged, not merely renamed). The old header +(preserved at +`_spidermonkey_install.orig-136a1-backup/include/mozjs-136a1/mozilla/Unused.h` +from the previous build) shows `Unused << expr` was only ever a thin +"suppress unused-nodiscard-return-value warning" helper +(`template void operator<<(const T&) const {}`) — functionally identical +to a plain `(void)expr;` cast. Two use sites in `JobQueue.cc` (the only file +in this codebase that used it) were switched to that, and the now-dead +`#include ` removed: + +```diff +- mozilla::Unused << finalizationRegistryCallbacks->append(callback); ++ (void)finalizationRegistryCallbacks->append(callback); +... +- mozilla::Unused << JS_CallFunction(cx, NULL, func, JS::HandleValueArray::empty(), &unused_rval); ++ (void)JS_CallFunction(cx, NULL, func, JS::HandleValueArray::empty(), &unused_rval); +``` + +### 9. `include/JobQueue.hh` / `src/JobQueue.cc` — off-thread dispatch API redesign (moderate risk) + +Next build errors after fix #8, all in the same area: `JS::InitDispatchToEventLoop` +no longer exists ("did you mean 'dispatchToEventLoop'?"); a 3-argument call +where only 2 are now expected; and `'run' is a protected member of +'JS::Dispatchable'`. Read the current `js/public/Promise.h` (lines 622-817) in +full to understand the new shape rather than guessing from the error text +alone. + +**What changed:** +- `JS::InitDispatchToEventLoop(cx, callback, closure)` → replaced by + `JS::InitAsyncTaskCallbacks(cx, dispatchCallback, delayedDispatchCallback, + asyncTaskStartedCallback, asyncTaskFinishedCallback, closure)`. The first + two callbacks are now both mandatory (previously only one existed at all); + the last two are optional (`nullptr` accepted). +- `DispatchToEventLoopCallback`'s signature changed from taking a raw + `JS::Dispatchable*` to taking ownership via `js::UniquePtr&&`. +- `Dispatchable::run()` is now `protected`. The new public entry point is the + static `Dispatchable::Run(JSContext*, js::UniquePtr&&, + MaybeShuttingDown)`, which takes ownership and is responsible for both + calling `run()` and cleaning up. +- A brand new, previously-nonexistent-for-this-embedding + `DelayedDispatchToEventLoopCallback` is now mandatory too. + +**Fix, in `src/JobQueue.cc`:** + +```diff +- JS::InitDispatchToEventLoop(cx, dispatchToEventLoop, cx); ++ JS::InitAsyncTaskCallbacks(cx, dispatchToEventLoop, delayedDispatchToEventLoop, nullptr, nullptr, cx); +``` + +`dispatchToEventLoop` itself: the raw `Dispatchable*` this used to smuggle +through a Python closure (packed as a `PyLong` pointer, same trick used +elsewhere in this file for the microtask fix in #7) is now obtained via +`dispatchable.release()` before packing, and reconstructed with +`js::UniquePtr(dispatchable)` on the other side, then run +via `JS::Dispatchable::Run(cx, ..., JS::Dispatchable::NotShuttingDown)` +instead of the old direct `dispatchable->run(cx, ...)` call — mechanical +translation of the ownership-transfer model, not a judgment call. + +**`delayedDispatchToEventLoop` — NEEDS REVIEW, the one genuine judgment call +in this fix:** this embedding has no existing mechanism for scheduling a +callback *safely from an arbitrary SpiderMonkey helper thread* with a delay. +`PyEventLoop::enqueueWithDelay` exists and is used elsewhere (JS +`setTimeout`), but it calls `asyncio.loop.call_later`, which — unlike +`call_soon_threadsafe` (used by `PyEventLoop::enqueue`, and safe from any +thread) — is not documented as callable from a thread other than the one +running the loop. Since `DelayedDispatchToEventLoopCallback` is explicitly +documented as needing to be safe from any thread, reusing `enqueueWithDelay` +directly would be a plausible new thread-safety bug, not a fix. + +Instead, this implementation always returns `false`, which +`js/public/Promise.h` explicitly sanctions: *"If a timeout manager is not +available for given context, it should return false."* + +**Correction made during this fix, left visible because the first instinct +was wrong and it's a useful lesson for reviewers:** the first attempt had +this call `dispatchable.release()` then `task->transferToRuntime()` directly, +based on a doc comment on `Dispatchable::transferToRuntime()` showing that +exact usage pattern. That failed to compile — `transferToRuntime()` is +`protected`, so an embedder callback has no access to it (the doc comment +describes SpiderMonkey's *own* internal usage, not the embedder-facing API). +The actually-correct, embedder-facing call was found by reading real +production code instead of inferring from a header comment: Gecko's own +`dom/workers/RuntimeService.cpp` (`JSDispatchableRunnable::PostDispatch`) +handles exactly this "took ownership, failed/declined to dispatch" case with +the public static `JS::Dispatchable::ReleaseFailedTask(std::move(task))`, +which is what this fix now uses. + +**Risk assessment**: this should only affect internal SpiderMonkey features +that specifically need an off-thread *delayed* dispatch (the header mentions +`Atomics.waitAsync` timeouts as an example). Ordinary JS `setTimeout` / +`setInterval` go through a separate, unaffected, already-working path +(pythonmonkey's own JS-exposed timer functions calling +`PyEventLoop::enqueueWithDelay` directly, on the main thread). **Not verified +against a real `Atomics.waitAsync`-with-timeout test case** — if this +embedding's use cases ever depend on that specific feature, this will need +a real timeout-manager implementation instead of the `false` stub. + +### 10. `src/modules/pythonmonkey/pythonmonkey.cc` — asm.js support removed, mechanical fix (low risk) + +Next build error after fix #9 (and the first one outside `JobQueue.cc`/`.hh`): +`error: no member named 'setAsmJS' in 'JS::ContextOptions'`. Confirmed via +`js/public/ContextOptions.h` that no asm.js-related member exists on +`ContextOptions` anymore at all — not renamed, removed. asm.js was a +pre-WebAssembly, Firefox-specific JS-subset compilation target; WebAssembly +(enabled separately via the still-present `.setWasm(true)`, unaffected by +this) has long since superseded it upstream. Simply deleted the +`.setAsmJS(true)` call in the `ContextOptionsRef` chaining call during +context setup — nothing to replace it with, since the feature itself is gone, +not relocated. + +### 11. `src/JSFunctionProxy.cc` / `src/JSMethodProxy.cc` — a 4th missing JobQueue checkpoint, found by independent retesting (moderate risk, now fixed) + +**This section exists because the "All passed, repeatably" claim under point 3 +of the Testing section below was wrong when first written.** Independent +retesting (by Claude, at the requester's request, specifically to audit this +PR before review) redeployed the actual built `pythonmonkey.pyd` + +`mozjs-157a1.dll` — the previously-installed copy in `site-packages` was +stale, still linked against the old `136a1` engine, so earlier manual smoke +tests after the JobQueue rewrite had not actually been exercising this build +— and found that awaiting a JS Promise resolved via `setTimeout` hangs +indefinitely, and so does the real `dcp_local_job_test.py` end-to-end job +(it gets through bootstrap and identity loading, then never fires a single +`readystatechange` event). + +**Root cause**: fix #7's checkpoint list (`JobQueue::runJobs`, +`PromiseType::getPyObject`, `futureOnDoneCallback` — three places new jobs +get enqueued into `cx->microTaskQueues` outside of a top-level +`JS_ExecuteScript()` call) missed a fourth: `JSFunctionProxy_call` +(`src/JSFunctionProxy.cc`) and `JSMethodProxy_call` (`src/JSMethodProxy.cc`) +are the generic entry points Python uses to call back into *any* JS function +or bound method it was handed — this is what fires a `setTimeout` callback +dispatched from `PyEventLoop`, or a JS event listener invoked directly from +Python code. Both call `JS_CallFunctionValue` and return without ever +draining the job queue afterward. If the JS function just called +resolved/rejected a Promise with already-attached reactions (the common case: +`resolve(...)` inside a `setTimeout` callback), that enqueues a job nothing +was scheduled to drain. + +**Fix**, identical in both files — add the same checkpoint used everywhere +else in this rewrite, immediately after the call succeeds: + +```diff + if (!JS_CallFunctionValue(cx, thisObj, jsFunc, jsArgs, &jsReturnVal)) { + setSpiderMonkeyException(cx); + return NULL; + } + ++ js::RunJobs(cx); ++ + if (PyErr_Occurred()) { + return NULL; + } +``` + +(`#include ` added to both files for the declaration, matching +`JobQueue.cc`'s existing include.) + +**Retested after this fix** — all of Testing point 3 below plus an added +sequential-delayed-promises case, and all of points 4 and 5 (the full +`localExec()` suite and the real `exec()` test) were rerun end-to-end against +this exact rebuilt binary. All passed; see the corrected Testing section +below. This is the second time in this same JobQueue rewrite that "compiles +and a few manual checks look right" turned out not to mean "actually works" +— treat that as a standing warning for any *other* not-yet-exercised path in +this rewrite (the Debugger-API paths flagged in "Not yet done" below), not +just the two paths that have now each independently failed once. + +--- + +## Testing — what was actually run, and what it showed + +Ten build errors were fixed in total (sections 1-10 above), each one a real +SpiderMonkey-internal API break between the old `136a1` nightly-alpha build +and current mozilla-central — none were environment/tooling issues by this +point (those were resolved earlier, before section 1). The build finally +succeeded (`pythonmonkey.pyd` + `mozjs-157a1.dll`, `BUILD_EXIT_CODE=0`). + +After that, four rounds of runtime verification were run (not just "it +compiles"): + +1. **Basic eval**: `pm.eval('1 + 2')`, `pm.eval('JSON.stringify(...)')` — + passed. +2. **`SharedArrayBuffer`/`Atomics`** — the original, one-line motivating fix + for this entire rebuild (a completely separate, older issue from + everything in this document). Verified still working: + `Atomics.store`/`Atomics.load` round-trip through a `SharedArrayBuffer` + returned the correct value. +3. **Async/Promise interop across the Python/JS boundary** — this is where + real bugs actually turned up (see the "UPDATE" note under fix #7 above for + the full story: a single `await` of a JS Promise hung indefinitely on the + first attempt, root-caused to `js::RunJobs(cx)` only being called from one + of the three places new jobs actually get enqueued). After fixing all + three call sites, verified: a single `await` of an already-resolved JS + Promise; a two-`await` chain inside a JS async function, checking both + completion *and* correct ordering (`[1, 3, 5]`, not e.g. `[1, 5, 3]`); and + a `setTimeout`-based Promise. **This third case was reported as passing + here, but that was wrong** — see fix #11 above: the actual built binary + deployed to `site-packages` was stale at the time (still the old `136a1` + engine), so this hadn't really been exercised against this rewrite. Once + retested against the real binary, the `setTimeout` case hung, was + root-caused to a 4th missing checkpoint (fix #11), and after that fix, all + of the above — plus an added sequential-back-to-back-delayed-promises + case — passed, repeatably, confirmed against the actual rebuilt + `pythonmonkey.pyd`. +4. **The real `localExec()` test suite**, run end-to-end against the new + engine, exactly as originally planned. **Like point 3, this was also + re-verified after fix #11** — `dcp_local_job_test.py` specifically hangs + after identity loading without that fix (identity loading itself is an + `async` JS function call, which goes through `PromiseType::getPyObject` + and was already covered; the job's own event/timer-driven machinery is + what hit the missing 4th checkpoint): + - `dcp_local_job_test.py` — a real job (`dcp.compute_for` over 8 letters, + uppercasing work function), through the full `localExec()` pipeline + (readystate transitions, identity loading from a real `id.keystore`, + job deployment, slice completion, result collection). **Passed** — + correct output `YELLING!`. + - `pycomod_localexec_test.py` — a much heavier stress test: real + filesystem shipping (`job.fs.add`) of a local Python package into the + sandbox, extra declared Pyodide modules (`pandas` on top of the usual + `numpy`/`cloudpickle`), extra work-function arguments flowing through + `job.jobArguments`, and — importantly — non-primitive slice results + (nested dicts of numpy arrays), which exercises cloudpickle's real + serialization round-trip rather than the primitive fast path. **Passed** + — all 5 slices completed with correct structure and correct numeric + values (spot-checked a sample series: `values[:5] = [25. 25. 25. 25. + 25.]`, `dtype=float32`, as expected for this model). Takes a few minutes + (real Pyodide package loading: `pandas`/`numpy`/`cloudpickle`/etc.) — + don't mistake the lack of output during that window for a hang. + +5. **Real `job.exec()` (not `localExec()`) — genuine network dispatch, + verified separately** (`dcp_real_exec_test.py`, modelled directly on + `dcp_sample_job.py`'s pattern but loading identity from `id.keystore` + the same safe way `dcp_local_job_test.py` does, rather than an inline + private key). This is a materially different code path from everything + above: `localExec()` never leaves the process, while `exec()` submits to + the real DCP scheduler, needs a funded wallet, and depends on real + workers actually being present on the target compute group + (`demo`/`dcp`, the same public demo group both existing sample scripts + already use). **Passed, real end-to-end**: full real readystate + lifecycle (`exec → init → preauth → deploying → listeners → + compute-groups → uploading → deployed`), a real scheduler-assigned job + ID, 8 real `result` events from real workers, no `nofunds`/`error` + events, correct final output `YELLING!`. This confirms the rebuild is + solid for the actual production dispatch path, not just the + local-simulation path this document otherwise focuses on. **Also + re-verified after fix #11**, for the same reason as point 4. + +## Not yet done / open as of this writing + +- **Resolved, was previously unverified**: fix #11 above closed the 4th + missing JobQueue checkpoint. Before it, every claim in the Testing section + that touched an event/timer-driven callback (the `setTimeout`-Promise case + in point 3, and both `localExec()`/`exec()` end-to-end tests in points 4-5) + had actually been checked against a stale, pre-rewrite binary rather than + this PR's real build, and would have hung for anyone who ran them for + real. All were rerun against the actual rebuilt `pythonmonkey.pyd` and now + pass. Leaving this note here rather than deleting it: if a *fifth* + Python→JS callback path turns up somewhere that neither this fix nor the + original three cover, that would make two independent misses in the same + rewrite, which would be a good reason to stop patching call sites + one-by-one and instead audit every `JS_Call*`/`JS_Invoke` call in the + codebase for the same gap systematically. + +- **Not independently verified**: `Atomics.waitAsync` with a real timeout + (the `delayedDispatchToEventLoop` stub in fix #9 always declines these — + see that section's risk assessment). Only relevant if something in this + codebase's dependency tree actually uses that specific API; not exercised + by either test suite above. +- **Not independently verified**: the Debugger-API-facing parts of the + JobQueue rewrite (`saveJobQueue`/`SavedJobQueue`, `isDrainingStopped`, + the debug microtask queue via `useDebugQueue`) — pythonmonkey doesn't + currently expose SpiderMonkey's Debugger API to Python, so these paths + are believed unreachable in normal use, but that belief hasn't been + tested against actually invoking the Debugger API. +- **Not independently verified**: the `BufferType.cc` GC-safety reasoning + (fix #5, `JS_GetArrayBufferViewFixedData` → `JS_GetArrayBufferViewData` + + `AutoAssertNoGC`) — this needs either a compacting/moving-GC stress test + (`--enable-gczeal` or equivalent) or review by someone with real + SpiderMonkey GC internals expertise before being trusted beyond + experimentation. Both test suites above exercise TypedArray/buffer code + paths incidentally (numpy arrays flow through cloudpickle, not directly + through this code path) but do not specifically stress-test this. +- **Not run**: any long-running / soak test. Everything above is a single + run of each script; no repeated-execution, memory-leak, or + long-session-stability testing has been done. The heap-allocated + `JS::PersistentRooted` objects created per-microtask in fix #7 + (`JobQueue::runJobs`) are freed on the happy path (`runMicroTaskCallback`) + but **not** on at least one error path worth double-checking before a + soak test: if `PyEventLoop::getRunningLoop()` fails inside + `JobQueue::runJobs` after a `rootedJob` has been allocated, it is deleted + correctly (see that code) — but this exact path has not been exercised + by any test above, since the running loop was always available. +- **Not reviewed by anyone else.** Every fix in this document was made by + one engineer (with AI pair-programming assistance) working from primary + sources (the actual SpiderMonkey headers and, where embedder-facing + behavior was unclear, real production usage in Gecko's own source) rather + than guessing, and where a first attempt was wrong (see fix #9's + `transferToRuntime`/`ReleaseFailedTask` correction, and fix #7's + three-checkpoint hang), that's recorded rather than smoothed over — but + none of it has had a second, independent pair of eyes. Recommended before + shipping this beyond internal experimentation: a real code review of + sections 5, 7, and 9 in particular (the three sections marked NEEDS + REVIEW / moderate-or-higher risk above), ideally by someone with prior + SpiderMonkey embedding experience. +- The old `136a1` install and DLL were preserved as `*.orig-136a1-backup` / + `*.orig-backup` throughout this work (see individual sections above) — + don't delete these until the team has independently confirmed the new + build in their own environment, not just this one. diff --git a/include/JobQueue.hh b/include/JobQueue.hh index 36734f92..4c8dbc7d 100644 --- a/include/JobQueue.hh +++ b/include/JobQueue.hh @@ -49,43 +49,39 @@ bool init(JSContext *cx); * If any error happens while generating the host defined data, this method * should set a pending exception to `cx` and return `false`. */ -bool getHostDefinedData(JSContext *cx, JS::MutableHandle data) const override; +bool getHostDefinedData(JSContext *cx, JS::MutableHandle incumbentGlobal, JS::MutableHandle data) const override; /** - * @brief Enqueue a reaction job `job` for `promise`, which was allocated at - * `allocationSite`. Provide `incumbentGlobal` as the incumbent global for - * the reaction job's execution. + * @brief Ask the embedding for the host defined global to use when running + * a JS microtask. * - * `promise` can be null if the promise is optimized out. - * `promise` is guaranteed not to be optimized out if the promise has - * non-default user-interaction flag. + * Same "we don't track this" stance as getHostDefinedData() above -- falls + * back to SpiderMonkey's own default, matching the reference embedding + * (InternalJobQueue::getHostDefinedGlobal, js/src/vm/JSContext.cpp). */ -bool enqueuePromiseJob(JSContext *cx, JS::HandleObject promise, - JS::HandleObject job, JS::HandleObject allocationSite, - JS::HandleObject incumbentGlobal) override; +bool getHostDefinedGlobal(JSContext *cx, JS::MutableHandle out) const override; /** - * @brief Run all jobs in the queue. Running one job may enqueue others; continue to - * run jobs until the queue is empty. + * @brief Pull every job SpiderMonkey has queued internally since the last + * call, and forward each one to the Python event-loop for execution. + * + * SpiderMonkey no longer pushes promise jobs to the embedding as they're + * created (the old enqueuePromiseJob); it queues them internally and + * expects the embedder to pull them here on demand, via the free function + * js::RunJobs(cx) (jsfriendapi.h -- not the same thing as this method: it's + * what calls cx->jobQueue->runJobs(cx)). PythonMonkey calls + * js::RunJobs(GLOBAL_CX) after every top-level JS_ExecuteScript(), plus a + * few call sites where JS callbacks resolve promises outside of script + * execution (see JSFunctionProxy.cc, PromiseType.cc). * * Calling this method at the wrong time can break the web. The HTML spec * indicates exactly when the job queue should be drained (in HTML jargon, * when it should "perform a microtask checkpoint"), and doing so at other * times can incompatibly change the semantics of programs that use promises * or other microtask-based features. - * - * This method is called only via AutoDebuggerJobQueueInterruption, used by - * the Debugger API implementation to ensure that the debuggee's job queue is - * protected from the debugger's own activity. See the comments on - * AutoDebuggerJobQueueInterruption. */ void runJobs(JSContext *cx) override; -/** - * @return true if the job queue is empty, false otherwise. - */ -bool empty() const override; - /** * @return true if the job queue stopped draining, which results in `empty()` being false after `runJobs()`. */ @@ -127,11 +123,34 @@ js::UniquePtr saveJobQueue(JSContext *) override; * @brief The callback for dispatching an off-thread promise to the event loop * see https://hg.mozilla.org/releases/mozilla-esr102/file/tip/js/public/Promise.h#l580 * https://hg.mozilla.org/releases/mozilla-esr102/file/tip/js/src/vm/OffThreadPromiseRuntimeState.cpp#l160 + * + * Takes ownership of the Dispatchable (run via the public static + * Dispatchable::Run, since Dispatchable::run() is protected). + * * @param closure - closure, currently the javascript context - * @param dispatchable - Pointer to the Dispatchable to be called + * @param dispatchable - the Dispatchable to be called; ownership transferred to this callback * @return not shutting down */ -static bool dispatchToEventLoop(void *closure, JS::Dispatchable *dispatchable); +static bool dispatchToEventLoop(void *closure, js::UniquePtr &&dispatchable); + +/** + * @brief The callback for dispatching an off-thread promise to the event + * loop after a delay. + * + * Always returns false (no timeout manager available), which + * js/public/Promise.h documents as a valid response when the embedding + * can't service delayed cross-thread dispatch. Only affects SpiderMonkey + * features needing a delayed off-thread callback (e.g. an + * Atomics.waitAsync timeout) -- ordinary setTimeout/setInterval go through + * PyEventLoop::enqueueWithDelay instead and are unaffected. NEEDS REVIEW: + * not verified against a real Atomics.waitAsync-with-timeout case. + * + * @param closure - closure, currently the javascript context + * @param dispatchable - the Dispatchable that would be called; ownership transferred to this callback + * @param delay - requested delay in milliseconds + * @return false (no timeout manager available for cross-thread delayed dispatch) + */ +static bool delayedDispatchToEventLoop(void *closure, js::UniquePtr &&dispatchable, uint32_t delay); /** * @brief The callback that gets invoked whenever a Promise is rejected without a rejection handler (uncaught/unhandled exception) diff --git a/mozcentral.version b/mozcentral.version index 55aeecbf..2373436c 100644 --- a/mozcentral.version +++ b/mozcentral.version @@ -1 +1 @@ -6bca861985ba51920c1cacc21986af01c51bd690 +1704651e7d6c706fcb753adab577e0954d61cee0 diff --git a/setup.sh b/setup.sh index 68560ade..2aa03864 100755 --- a/setup.sh +++ b/setup.sh @@ -19,37 +19,58 @@ elif [[ "$OSTYPE" == "darwin"* ]]; then # macOS brew update || true # allow failure brew install cmake pkg-config wget unzip coreutils # `coreutils` installs the `realpath` command brew install lld -elif [[ "$OSTYPE" == "msys"* ]]; then # Windows +elif [[ "$OSTYPE" == "msys"* || "$OSTYPE" == "cygwin"* ]]; then # Windows echo "Dependencies are not going to be installed automatically on Windows." else echo "Unsupported OS" exit 1 fi # Install rust compiler -echo "Installing rust compiler" -unset HOST_ABI_FLAGS -if [[ "$OSTYPE" == "msys"* ]]; then # Windows - HOST_ABI_FLAGS=("--default-host" "$(clang --print-target-triple)") +# Skip if already installed: re-running rustup-init.sh here downloads and +# runs a fresh installer exe, which Defender/SmartScreen blocks on this box. +if command -v rustup >/dev/null && rustup toolchain list 2>/dev/null | grep -q '^1\.85'; then + echo "Rust 1.85 toolchain already installed, skipping rustup-init" +else + echo "Installing rust compiler" + unset HOST_ABI_FLAGS + if [[ "$OSTYPE" == "msys"* || "$OSTYPE" == "cygwin"* ]]; then # Windows + HOST_ABI_FLAGS=("--default-host" "$(clang --print-target-triple)") + fi + curl --proto '=https' --tlsv1.2 https://raw.githubusercontent.com/rust-lang/rustup/refs/tags/1.28.2/rustup-init.sh -sSf | sh -s -- -y ${HOST_ABI_FLAGS+"${HOST_ABI_FLAGS[@]}"} --default-toolchain 1.85 fi -curl --proto '=https' --tlsv1.2 https://raw.githubusercontent.com/rust-lang/rustup/refs/tags/1.28.2/rustup-init.sh -sSf | sh -s -- -y ${HOST_ABI_FLAGS+"${HOST_ABI_FLAGS[@]}"} --default-toolchain 1.85 CARGO_BIN="$HOME/.cargo/bin/cargo" # also works for Windows. On Windows this equals to %USERPROFILE%\.cargo\bin\cargo -$CARGO_BIN install cbindgen +command -v cbindgen >/dev/null || $CARGO_BIN install cbindgen # Setup Poetry -echo "Installing poetry" -curl -sSL https://install.python-poetry.org | python3 - --version "1.7.1" -if [[ "$OSTYPE" == "msys"* ]]; then # Windows +if [[ "$OSTYPE" == "msys"* || "$OSTYPE" == "cygwin"* ]]; then # Windows POETRY_BIN="$APPDATA/Python/Scripts/poetry" else POETRY_BIN="$HOME/.local/bin/poetry" fi -$POETRY_BIN self add 'poetry-dynamic-versioning[plugin]' +# Skip if already installed (same idempotency reasoning as rustup above). +# Falls back to `python` since this machine has no `python3` on PATH. +# Poetry is still needed below by the .git/hooks/pre-commit branch. +if command -v "$POETRY_BIN" >/dev/null || [ -x "$POETRY_BIN" ]; then + echo "Poetry already installed, skipping" +else + echo "Installing poetry" + PYTHON_FOR_POETRY=$(command -v python3 || command -v python) + curl -sSL https://install.python-poetry.org | "$PYTHON_FOR_POETRY" - --version "1.7.1" + "$POETRY_BIN" self add 'poetry-dynamic-versioning[plugin]' +fi echo "Done installing dependencies" echo "Downloading spidermonkey source code" # Read the commit hash for mozilla-central from the `mozcentral.version` file MOZCENTRAL_VERSION=$(cat mozcentral.version) -wget -c -q -O firefox-source-${MOZCENTRAL_VERSION}.zip https://github.com/mozilla-firefox/firefox/archive/${MOZCENTRAL_VERSION}.zip -unzip -q firefox-source-${MOZCENTRAL_VERSION}.zip && mv firefox-${MOZCENTRAL_VERSION} firefox-source +# Skip if already extracted -- lets this script be re-run after a later +# step fails without re-downloading/re-extracting every time. +if [ ! -d firefox-source ]; then + # curl instead of wget: wget.exe is blocked by this machine's WDAC policy. + curl -fsSL -o firefox-source-${MOZCENTRAL_VERSION}.zip https://github.com/mozilla-firefox/firefox/archive/${MOZCENTRAL_VERSION}.zip + unzip -q firefox-source-${MOZCENTRAL_VERSION}.zip && mv firefox-${MOZCENTRAL_VERSION} firefox-source +else + echo "firefox-source already exists, skipping download+extract" +fi echo "Done downloading spidermonkey source code" echo "Building spidermonkey" @@ -69,6 +90,7 @@ sed -i'' -e '/MOZ_CRASH_UNSAFE_PRINTF/,/__PRETTY_FUNCTION__);/d' ./mfbt/LinkedLi sed -i'' -e '/MOZ_ASSERT(stackRootPtr == nullptr);/d' ./js/src/vm/JSContext.cpp # would assert false in Debug Build since we extensively use `new JS::Rooted` sed -i'' -e 's/"-fuse-ld=ld"/"-ld64" if c_compiler.version > "14.0.0" else "-fuse-ld=ld"/' ./build/moz.configure/toolchain.configure # XCode 15 changed the linker behaviour. See https://developer.apple.com/documentation/xcode-release-notes/xcode-15-release-notes#Linking sed -i'' -e 's/defined(XP_WIN)/defined(_WIN32)/' ./mozglue/baseprofiler/public/BaseProfilerUtils.h # this header file is introduced to js/Debug.h in https://phabricator.services.mozilla.com/D221102, but it would be compiled without XP_WIN in this building configuration +sed -i'' -e 's/os\.environ\["MOZILLABUILD"\]/os.environ.get("MOZILLABUILD", "")/g' ./python/mozbuild/mozbuild/backend/visualstudio.py # avoid KeyError: we don't use the official Mozilla Build package, so this is never set cd js/src mkdir -p _build @@ -77,16 +99,14 @@ mkdir -p ../../../../_spidermonkey_install/ ../configure --target=$(clang --print-target-triple) \ --prefix=$(realpath $PWD/../../../../_spidermonkey_install) \ --with-intl-api \ - $(if [[ "$OSTYPE" != "msys"* ]]; then echo "--without-system-zlib"; fi) \ + $(if [[ "$OSTYPE" != "msys"* && "$OSTYPE" != "cygwin"* ]]; then echo "--without-system-zlib"; fi) \ --disable-debug-symbols \ --disable-jemalloc \ --disable-tests \ $(if [[ "$OSTYPE" == "darwin"* ]]; then echo "--enable-linker=ld64"; fi) \ - --enable-optimize \ - --disable-explicit-resource-management -# disable-explicit-resource-management: Disable the `using` syntax that is enabled by default in SpiderMonkey nightly, otherwise the header files will disagree with the compiled lib .so file -# when it's using a `IF_EXPLICIT_RESOURCE_MANAGEMENT` macro, e.g., the `enum JSProtoKey` index would be off by 1 (header `JSProto_Uint8Array` 27 will be interpreted as `JSProto_Int8Array` in lib as lib has an extra element) -# https://bugzilla.mozilla.org/show_bug.cgi?id=1940342 + --enable-optimize +# --disable-explicit-resource-management (worked around Bugzilla 1940342) +# is no longer a recognized flag -- the feature it gated has since shipped. make -j$CPUS echo "Done building spidermonkey" @@ -120,7 +140,7 @@ if test -f .git/hooks/pre-commit; then cd uncrustify-source mkdir -p build cd build - if [[ "$OSTYPE" == "msys"* ]]; then # Windows + if [[ "$OSTYPE" == "msys"* || "$OSTYPE" == "cygwin"* ]]; then # Windows cmake ../ cmake --build . -j$CPUS --config Release cp Release/uncrustify.exe ../../uncrustify.exe diff --git a/src/BufferType.cc b/src/BufferType.cc index f0726bce..40926ee7 100644 --- a/src/BufferType.cc +++ b/src/BufferType.cc @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -88,9 +89,23 @@ PyObject *BufferType::fromJsTypedArray(JSContext *cx, JS::HandleObject typedArra bool isSharedMemory; if (!JS_GetArrayBufferViewBuffer(cx, typedArray, &isSharedMemory)) return nullptr; - uint8_t __destBuf[0] = {}; // we don't care about its value as it's used only if the TypedArray still having inline data - uint8_t *data = JS_GetArrayBufferViewFixedData(typedArray, __destBuf, 0 /* making sure we don't copy inline data */); - if (data == nullptr) { // shared memory or still having inline data + if (isSharedMemory) { + PyErr_SetString(PyExc_TypeError, "PythonMonkey cannot coerce TypedArrays backed by shared memory."); + return nullptr; + } + + // NEEDS REVIEW: JS_GetArrayBufferViewFixedData was removed upstream; its + // replacement trades the old "return nullptr if data is still inline/ + // movable" runtime guard for a caller-supplied no-GC token. Safety here + // relies on JS_GetArrayBufferViewBuffer() above having already promoted + // any inline data to a stable allocation -- not independently verified + // against SpiderMonkey's GC (e.g. via --enable-gczeal). AutoAssertNoGC, + // not the base AutoRequireNoGC (protected ctor), since it actually + // asserts at runtime in diagnostic builds instead of being a bare marker. + JS::AutoAssertNoGC nogc(cx); + bool isSharedMemory2; // redundant with isSharedMemory above; required by this function's signature + uint8_t *data = static_cast(JS_GetArrayBufferViewData(typedArray, &isSharedMemory2, nogc)); + if (data == nullptr) { PyErr_SetString(PyExc_TypeError, "PythonMonkey cannot coerce TypedArrays backed by shared memory."); return nullptr; } diff --git a/src/JSFunctionProxy.cc b/src/JSFunctionProxy.cc index 99a32552..88bcecba 100644 --- a/src/JSFunctionProxy.cc +++ b/src/JSFunctionProxy.cc @@ -16,6 +16,7 @@ #include "include/setSpiderMonkeyException.hh" #include +#include #include @@ -59,6 +60,11 @@ PyObject *JSFunctionProxyMethodDefinitions::JSFunctionProxy_call(PyObject *self, return NULL; } + // This is the generic entry point for any Python->JS callback (e.g. a + // setTimeout callback), so a Promise resolved here has nothing else + // scheduled to drain its reaction jobs. See JobQueue.cc's runJobs. + js::RunJobs(cx); + if (PyErr_Occurred()) { return NULL; } diff --git a/src/JSMethodProxy.cc b/src/JSMethodProxy.cc index 78e1189b..ad0ada61 100644 --- a/src/JSMethodProxy.cc +++ b/src/JSMethodProxy.cc @@ -16,6 +16,7 @@ #include "include/setSpiderMonkeyException.hh" #include +#include #include @@ -70,6 +71,9 @@ PyObject *JSMethodProxyMethodDefinitions::JSMethodProxy_call(PyObject *self, PyO return NULL; } + // Same checkpoint as JSFunctionProxy_call, for bound methods. + js::RunJobs(cx); + if (PyErr_Occurred()) { return NULL; } diff --git a/src/JobQueue.cc b/src/JobQueue.cc index 928746fd..a2008c9c 100644 --- a/src/JobQueue.cc +++ b/src/JobQueue.cc @@ -14,11 +14,12 @@ #include "include/PyEventLoop.hh" #include "include/pyTypeFactory.hh" #include "include/PromiseType.hh" +#include "include/setSpiderMonkeyException.hh" #include #include -#include +#include #include @@ -26,41 +27,93 @@ JobQueue::JobQueue(JSContext *cx) { finalizationRegistryCallbacks = new JS::PersistentRooted(cx); // Leaks but it's OK since freed at process exit } -bool JobQueue::getHostDefinedData(JSContext *cx, JS::MutableHandle data) const { +bool JobQueue::getHostDefinedData(JSContext *cx, JS::MutableHandle incumbentGlobal, JS::MutableHandle data) const { + incumbentGlobal.set(nullptr); // We don't need the incumbent global data.set(nullptr); // We don't need the host defined data return true; // `true` indicates no error } -bool JobQueue::enqueuePromiseJob(JSContext *cx, - [[maybe_unused]] JS::HandleObject promise, - JS::HandleObject job, - [[maybe_unused]] JS::HandleObject allocationSite, - JS::HandleObject incumbentGlobal) { - - // Convert the `job` JS function to a Python function for event-loop callback - JS::RootedValue jobv(cx, JS::ObjectValue(*job)); - PyObject *callback = pyTypeFactory(cx, jobv); - - // Send job to the running Python event-loop - PyEventLoop loop = PyEventLoop::getRunningLoop(); - if (!loop.initialized()) return false; +bool JobQueue::getHostDefinedGlobal(JSContext *cx, JS::MutableHandle out) const { + out.set(nullptr); + return true; +} - // Inform the JS runtime that the job queue is no longer empty - JS::JobQueueMayNotBeEmpty(cx); +// The PyCFunction invoked by the Python event-loop once it's ready to run a +// single deferred JS microtask. `closure` is a 2-tuple of (JSContext*, +// JS::PersistentRooted* job), both smuggled through as PyLong +// pointers the same way JobQueue::dispatchToEventLoop's callDispatchFunc +// does below for JS::Dispatchable. +static PyObject *runMicroTaskCallback(PyObject *closure, PyObject *Py_UNUSED(unused)) { + JSContext *cx = (JSContext *)PyLong_AsVoidPtr(PyTuple_GetItem(closure, 0)); + auto *rootedJob = (JS::PersistentRooted *)PyLong_AsVoidPtr(PyTuple_GetItem(closure, 1)); + + JS::Rooted job(cx, rootedJob->get()); + delete rootedJob; // the PersistentRooted was only needed to keep `job` alive until now + + bool ok = true; + JSObject *global = JS::GetExecutionGlobalFromJSMicroTask(job); + if (global) { + JSAutoRealm ar(cx, global); + ok = JS::RunJSMicroTask(cx, job); + } - loop.enqueue(callback); + // Running this microtask may enqueue the next one in an await chain -- + // re-checkpoint so it doesn't just sit there. See also PromiseType.cc. + js::RunJobs(cx); - Py_DECREF(callback); - return true; + if (!ok) { + setSpiderMonkeyException(cx); + return NULL; // propagates as a Python exception; PyEventLoop's own + // eventLoopJobWrapper surfaces it to the loop's exception handler + } + Py_RETURN_NONE; } -void JobQueue::runJobs(JSContext *cx) { - // Do nothing -} +static PyMethodDef runMicroTaskCallbackDef = {"JsMicroTaskCallable", runMicroTaskCallback, METH_NOARGS, NULL}; -bool JobQueue::empty() const { - // TODO (Tom Tang): implement using `get_running_loop` and getting job count on loop??? - return true; // see https://hg.mozilla.org/releases/mozilla-esr128/file/tip/js/src/builtin/Promise.cpp#l6946 +// NEEDS REVIEW: GC-safety of rooting a JSMicroTask* across the gap between +// dequeuing it here and the Python event-loop calling runMicroTaskCallback +// is modelled on the finalizationRegistryCallbacks pattern below, but not +// independently verified for JSMicroTask specifically. Also unverified: +// that draining once per top-level JS_ExecuteScript() (pythonmonkey.cc) is +// the only place a checkpoint is needed for this embedding. +void JobQueue::runJobs(JSContext *cx) { + while (JS::HasAnyMicroTasks(cx)) { + JS::RootedValue entry(cx, JS::DequeueNextMicroTask(cx)); + if (entry.isNull()) { + break; + } + + JS::Rooted job(cx, JS::ToMaybeWrappedJSMicroTask(entry)); + if (!job) { + continue; // not a JS microtask; nothing we support runs these + } + + // Root the job on the heap so it survives until the Python event-loop + // calls back into us, which may be well after this function returns. + auto *rootedJob = new JS::PersistentRooted(cx, job); + + PyObject *cxArg = PyLong_FromVoidPtr(cx); + PyObject *jobArg = PyLong_FromVoidPtr(rootedJob); + PyObject *closure = PyTuple_Pack(2, cxArg, jobArg); + Py_DECREF(cxArg); + Py_DECREF(jobArg); + PyObject *callback = PyCFunction_New(&runMicroTaskCallbackDef, closure); + Py_DECREF(closure); + + PyEventLoop loop = PyEventLoop::getRunningLoop(); + if (!loop.initialized()) { + delete rootedJob; + Py_DECREF(callback); + return; + } + + // Inform the JS runtime that the job queue is no longer empty + JS::JobQueueMayNotBeEmpty(cx); + + loop.enqueue(callback); + Py_DECREF(callback); + } } bool JobQueue::isDrainingStopped() const { @@ -79,7 +132,9 @@ js::UniquePtr JobQueue::saveJobQueue(JSContext *cx) bool JobQueue::init(JSContext *cx) { JS::SetJobQueue(cx, this); - JS::InitDispatchToEventLoop(cx, dispatchToEventLoop, cx); + // Last two args (asyncTaskStarted/FinishedCallback) are optional; this + // embedding doesn't need to track background-task liveness. + JS::InitAsyncTaskCallbacks(cx, dispatchToEventLoop, delayedDispatchToEventLoop, nullptr, nullptr, cx); JS::SetPromiseRejectionTrackerCallback(cx, promiseRejectionTracker); return true; } @@ -87,13 +142,15 @@ bool JobQueue::init(JSContext *cx) { static PyObject *callDispatchFunc(PyObject *dispatchFuncTuple, PyObject *Py_UNUSED(unused)) { JSContext *cx = (JSContext *)PyLong_AsVoidPtr(PyTuple_GetItem(dispatchFuncTuple, 0)); JS::Dispatchable *dispatchable = (JS::Dispatchable *)PyLong_AsVoidPtr(PyTuple_GetItem(dispatchFuncTuple, 1)); - dispatchable->run(cx, JS::Dispatchable::NotShuttingDown); + // Dispatchable::run() is protected; reconstruct the UniquePtr released + // into raw form by dispatchToEventLoop() below and run it via Run(). + JS::Dispatchable::Run(cx, js::UniquePtr(dispatchable), JS::Dispatchable::NotShuttingDown); Py_RETURN_NONE; } static PyMethodDef callDispatchFuncDef = {"JsDispatchCallable", callDispatchFunc, METH_NOARGS, NULL}; -bool JobQueue::dispatchToEventLoop(void *closure, JS::Dispatchable *dispatchable) { +bool JobQueue::dispatchToEventLoop(void *closure, js::UniquePtr &&dispatchable) { JSContext *cx = (JSContext *)closure; // The `dispatchToEventLoop` function is running in a helper thread, so @@ -101,7 +158,10 @@ bool JobQueue::dispatchToEventLoop(void *closure, JS::Dispatchable *dispatchable // see https://docs.python.org/3/c-api/init.html#non-python-created-threads PyGILState_STATE gstate = PyGILState_Ensure(); - PyObject *dispatchFuncTuple = PyTuple_Pack(2, PyLong_FromVoidPtr(cx), PyLong_FromVoidPtr(dispatchable)); + // Release ownership into a raw pointer to smuggle it through the Python + // closure; reclaimed by callDispatchFunc via Dispatchable::Run above. + JS::Dispatchable *raw = dispatchable.release(); + PyObject *dispatchFuncTuple = PyTuple_Pack(2, PyLong_FromVoidPtr(cx), PyLong_FromVoidPtr(raw)); PyObject *pyFunc = PyCFunction_New(&callDispatchFuncDef, dispatchFuncTuple); // Avoid using the current, JS helper thread to send jobs to event-loop as it may cause deadlock @@ -111,6 +171,15 @@ bool JobQueue::dispatchToEventLoop(void *closure, JS::Dispatchable *dispatchable return true; } +bool JobQueue::delayedDispatchToEventLoop(void *closure, js::UniquePtr &&dispatchable, uint32_t delay) { + // No cross-thread-safe delayed-dispatch mechanism here (see JobQueue.hh). + // ReleaseFailedTask is the embedder-facing way to decline after taking + // ownership -- transferToRuntime() is SpiderMonkey's own internal use + // and is protected. + JS::Dispatchable::ReleaseFailedTask(std::move(dispatchable)); + return false; +} + bool sendJobToMainLoop(PyObject *pyFunc) { PyGILState_STATE gstate = PyGILState_Ensure(); @@ -165,7 +234,8 @@ void JobQueue::promiseRejectionTracker(JSContext *cx, } void JobQueue::queueFinalizationRegistryCallback(JSFunction *callback) { - mozilla::Unused << finalizationRegistryCallbacks->append(callback); + // mozilla::Unused (mfbt) was removed upstream; it was just a discard cast. + (void)finalizationRegistryCallbacks->append(callback); } bool JobQueue::runFinalizationRegistryCallbacks(JSContext *cx) { @@ -179,7 +249,7 @@ bool JobQueue::runFinalizationRegistryCallbacks(JSContext *cx) { JS::RootedFunction func(cx, f); JS::RootedValue unused_rval(cx); // we don't raise an exception here because there is nowhere to catch it - mozilla::Unused << JS_CallFunction(cx, NULL, func, JS::HandleValueArray::empty(), &unused_rval); + (void)JS_CallFunction(cx, NULL, func, JS::HandleValueArray::empty(), &unused_rval); ranCallbacks = true; } diff --git a/src/PromiseType.cc b/src/PromiseType.cc index 5a3f94b3..52082450 100644 --- a/src/PromiseType.cc +++ b/src/PromiseType.cc @@ -78,6 +78,11 @@ PyObject *PromiseType::getPyObject(JSContext *cx, JS::HandleObject promise) { js::SetFunctionNativeReserved(onResolved, PROMISE_OBJ_SLOT, JS::ObjectValue(*promise)); JS::AddPromiseReactions(cx, promise, onResolved, onResolved); + // If `promise` was already settled, AddPromiseReactions just queued a job + // with nothing else scheduled to drain it (we're outside JS_ExecuteScript + // here). See JobQueue::runJobs. + js::RunJobs(cx); + return future.getFutureObject(); // must be a new reference, ref count == 3 // Here the ref count for the `future` object is 3, but will immediately decrease to 2 in `PyEventLoop::Future`'s destructor when the `PromiseType::getPyObject` function ends // Leaving one reference for the returned Python object, and another one for the `onResolved` callback function @@ -109,6 +114,11 @@ static PyObject *futureOnDoneCallback(PyObject *futureCallbackTuple, PyObject *a } else { // having exception set, to reject the promise JS::RejectPromise(cx, promise, JS::RootedValue(cx, jsTypeFactorySafe(cx, exception))); } + + // Same as getPyObject above: resolving/rejecting here may queue reaction + // jobs with nothing else scheduled to drain them. + js::RunJobs(cx); + Py_XDECREF(exception); // cleanup delete rootedPtr; // no longer needed to be rooted, clean it up diff --git a/src/modules/pythonmonkey/pythonmonkey.cc b/src/modules/pythonmonkey/pythonmonkey.cc index 8408b594..5314e44b 100644 --- a/src/modules/pythonmonkey/pythonmonkey.cc +++ b/src/modules/pythonmonkey/pythonmonkey.cc @@ -488,6 +488,10 @@ static PyObject *eval(PyObject *self, PyObject *args) { return NULL; } + // Mirrors the HTML spec's "clean up after running script" checkpoint -- + // see JobQueue::runJobs for why the embedder now has to drain this itself. + js::RunJobs(GLOBAL_CX); + // translate to the proper python type PyObject *returnValue = pyTypeFactory(GLOBAL_CX, rval); if (PyErr_Occurred()) { @@ -571,9 +575,10 @@ PyMODINIT_FUNC PyInit_pythonmonkey(void) return NULL; } + // asm.js was removed from SpiderMonkey (superseded by WebAssembly, set + // via .setWasm(true) below); ContextOptions::setAsmJS no longer exists. JS::ContextOptionsRef(GLOBAL_CX) .setWasm(true) - .setAsmJS(true) .setAsyncStack(true) .setSourcePragmas(true); @@ -594,6 +599,10 @@ PyMODINIT_FUNC PyInit_pythonmonkey(void) JS::AddGCNurseryCollectionCallback(GLOBAL_CX, nurseryCollectionCallback, NULL); JS::RealmCreationOptions creationOptions = JS::RealmCreationOptions(); + // Off by default (a Spectre mitigation for untrusted web content, which + // doesn't apply to this embedded single-process run); Pyodide's threaded + // WASM build otherwise fails to link ("shared memory is disabled"). + creationOptions.setSharedMemoryAndAtomicsEnabled(true); JS::RealmBehaviors behaviours = JS::RealmBehaviors(); JS::RealmOptions options = JS::RealmOptions(creationOptions, behaviours); static JSClass globalClass = {"global", JSCLASS_GLOBAL_FLAGS, &JS::DefaultGlobalClassOps};