fix(base): make user worker reuse code-aware - #736
Open
CesarManzoCode wants to merge 3 commits into
Open
Conversation
Adds a regression test that drives EdgeRuntime.userWorkers.create() through the main_eszip main worker with a fixed servicePath but two different eszip bundles (marker "A" then marker "B", then "A" again). Before the fix the pool keys warm-worker reuse solely by servicePath, so the second request is served by the worker still running bundle "A" and the assertion for "MARKER_B" fails - the runtime serves code that was never deployed for that request (supabase#721). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsXvAfDZbQvCVBYhq7L8Xx
WorkerPool keyed warm-worker reuse for a service path on the service path alone. Because the reuse lookup happens before a worker is built from the incoming WorkerContextInitOpts, an already-active worker was returned even when the caller supplied a different code artifact (a redeployed eszip bundle, different inline module code, or a different entrypoint). The new code was then silently ignored - see supabase#721. Introduce WorkerCodeIdentity: a deterministic, content-derived digest over the executable artifact (inline eszip bytes, inline module code) plus the service path and any explicit entrypoint. A pre-parsed eszip cannot be digested cheaply, so it maps to `Opaque` and is never eligible for reuse. The pool now stores the identity on each UserWorkerProfile and on the per-service ActiveWorkerRegistry, and: * maybe_active_worker() rejects the registry when its identity is incompatible with the request, retiring the stale workers so the caller proceeds through normal creation; * add_user_worker() reconciles the same way, so a concurrent create for a different artifact does not pool mismatched code under one key. Same artifact -> reuse still allowed. Different artifact -> the existing worker is retired via the normal retirement path and a fresh worker is built. Semaphore/concurrency, retirement, idle, shutdown, per_request and force_create behaviour are unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsXvAfDZbQvCVBYhq7L8Xx
Three pre-upstream hardening changes to WorkerCodeIdentity (introduced in the previous commit for supabase#721): * Replace the 64-bit xxh3 digest with a full SHA-256 (`[u8; 32]`, never truncated). Worker identity is a correctness boundary: two different executable artifacts colliding under a small non-cryptographic hash would let the pool hand back a worker running code that was never deployed for the request. `sha2` is already a workspace dependency, so this drops the `xxhash-rust` dependency from `ext_workers` rather than adding one. * Fold the effective import map path (`context.importMapPath`) into the digest. `crates/base/src/runtime` passes that path to the module loader both when it builds an eszip from filesystem/inline source and when it loads a pre-built eszip (via `MigrateOptions`), so the same source can resolve bare specifiers differently under a different import map. Two requests that agree on servicePath, entrypoint and source but disagree on importMapPath must not silently share a warm worker. Read through the existing `WorkerRuntimeOpts` context, no new configuration channel. * Document that filesystem source under `service_path` is intentionally outside the reuse identity: a plain file-backed worker keeps the pre-existing semantics (an on-disk edit needs `force_create`, exactly as before). The digest still improves that case - a changed entrypoint or import map path now forces a fresh worker - and never falls to `Opaque`, so no new stale-code path is created for a normal non-forceCreate caller. Tests: add import-map identity coverage and a full-SHA-256 known-answer test; the existing content-addressing unit test and the `test_user_worker_reuse_is_code_aware` end-to-end regression are unchanged in intent and still pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rFnYFfUJAPgKAX4ajrQ2t
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Bug fix.
Description
User workers are currently reused based on
servicePathalone.That means a subsequent
EdgeRuntime.userWorkers.create()call can provide adifferent executable artifact while still receiving an already-active worker
running the previous artifact.
This PR makes warm-worker reuse code-aware.
A worker is now reusable only when both its service path and executable identity
match the incoming request.
Executable identity is derived deterministically with SHA-256 from the
reuse-relevant inputs, including:
importMapPathPre-parsed ESZip values that cannot be deterministically identified are treated
as non-reusable.
When a request arrives with a different executable identity, incompatible active
workers for that service path are retired through the existing lifecycle path
and a fresh worker is created.
Same artifact → warm-worker reuse remains unchanged.
Different artifact → the previous worker cannot be silently reused.
Existing semaphore, retirement, idle cleanup, shutdown,
per_request,per_worker, andforceCreatebehavior is preserved.Regression
Added an end-to-end regression that uses one
servicePathwith two differentESZip bundles:
MARKER_AMARKER_BMARKER_AWithout the code-identity check, step 3 fails because the runtime reuses the
worker still serving
MARKER_A.With this change, the test passes.
Additional unit coverage verifies:
importMapPath→ incompatibleValidation
cargo fmt -p base -p ext_workers --checkcargo clippy -p ext_workers -p base --all-targets -- -D warningscargo test -p ext_workerscargo test -p base --test integration_tests test_user_worker_reuse_is_code_aware -- --test-threads=1All targeted checks pass.
Related
Related to #721.
This fixes a reproducible stale-worker reuse path in the open-source runtime.
The hosted multi-region propagation behavior reported in #721 involves
infrastructure outside this repository, so this PR intentionally does not claim
to explain the entire hosted incident.