feat(node,runtime-core,sdk): track and dispose evaluation-time side effects (#5031) - #5067
Open
justonemorenight wants to merge 15 commits into
Open
Conversation
…when cleanup misses removeRemote (used by registerRemotes with force: true) looked up the remote's runtime instance in __FEDERATION__.__INSTANCES__ only by the registered name. When that differs from the name the remote was built with, the lookup silently missed: the container global and module cache were dropped but the old instance stayed registered, so every forced re-registration leaked a full container graph. The lookup now falls back to matching by entryGlobalName (the runtime instance name for enhanced/webpack-built containers) and logs a warning when no instance can be found instead of failing silently. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Remote chunks fetched over HTTP by the node runtime plugin were executed via direct `eval` of a wrapper string. Functions created by direct eval capture the enclosing scope, so every module factory in the chunk kept the fetched `data` string alive in its closure context in addition to the flattened wrapper that V8 retains as script source. A live 3 MB chunk therefore cost 6 MB, and in long-running SSR hosts that re-execute remotes the duplicate copy came along with every generation. Introduce a shared `compileChunk` helper: on Node it compiles through `vm.Script` with the chunk URL as the filename (same code path the filesystem loader already used), and on runtimes without `vm` it uses `new Function`, which does not capture scope. The stringified `httpEvalStrategy` in the filesystem strategies gets the same treatment. Measured on a 3 MB chunk with a heap snapshot after GC: one chunk source string retained per live container instead of two, live-container heap 19.7 MB -> 16.7 MB, and 30 forced re-registrations stay flat at 17.7 MB. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every distinct script V8 compiles lands in an isolate-wide compilation cache holding its source text and compiled code. The cache never ages out on its own (40 full GCs and 400 MB of unrelated churn left it intact) and is evicted only when the heap approaches V8's own limit, which defaults from physical memory rather than the container's cgroup. A host that force-registers a genuinely new remote build on each refresh therefore grew about 6.4 MB per refresh with a 3 MB chunk, regardless of whether the chunk was compiled with eval, vm.Script, new Function or vm.compileFunction, and heap snapshots never showed it because writing a snapshot clears the cache. Wrap the two places that compile remote code, loadScriptNode in the sdk and compileChunk in the node runtime plugin, in withoutCompilationCache(), which flips --no-compilation-cache around the synchronous compile call and restores the flag afterwards. FEDERATION_KEEP_COMPILATION_CACHE=true opts out. Measured (3 MB chunk, unique build per forced refresh, 40 refreshes): heap 16.8 MB -> 18.0 MB with the change, 16.8 MB -> 272.9 MB with the opt-out. Identical-build refreshes are unchanged at 18.0 MB. An uncached 2 MB compile costs about 37 ms against 4 ms cached, paid once per remote execution. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…flag is unavailable --no-compilation-cache is not accepted in NODE_OPTIONS, so hosts that only control environment variables (Lambda-style runtimes) cannot use the startup flag, and runtimes without v8.setFlagsFromString cannot use the scoped toggle. Add FEDERATION_COMPILATION_CACHE=flag|gc|off: flag (default) keeps the scoped toggle; gc compiles normally and then runs one full garbage collection through the inspector once the compile burst settles, which clears the whole cache (71 ms on a small heap, ~540 ms on a 500 MB heap, against 28 s for a heap snapshot); off leaves the cache alone. gc is also the automatic fallback when setFlagsFromString is missing. The GC timer is unref'd and the inspector session is disconnected after post() returns, since disconnecting from inside the synchronous callback deadlocks the session. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…g the inspector session HeapProfiler.collectGarbage completes asynchronously; disconnecting right after post() cancelled it, which left the gc strategy ineffective. Disconnect on the next macrotask after the completion callback instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Node's permission model (--permission) denies inspector.Session.connect with ERR_ACCESS_DENIED. When that happens the gc strategy now uses an exposed gc() with V8's last-resort flavor, which also clears the compilation cache and can be enabled through NODE_OPTIONS=--expose-gc on env-only hosts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Drop the inspector-based and exposed-gc fallbacks; the compilation cache is handled solely by flipping --no-compilation-cache around the synchronous compile call, with FEDERATION_KEEP_COMPILATION_CACHE=true as the opt-out. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Separate compilation from policy with a single implementation in the sdk: `buildCommonJsWrapper` (wrapper shape), `compileCommonJsModule` (vm.Script when vm is obtainable, otherwise new Function; errors never retried on the other backend) and `withRemoteCompilationPolicy` (process-level manager for V8's compilation cache flag: shared globalThis depth counter, toggled only on 0->1 and 1->0, skipped when the process already runs with --no-compilation-cache or FEDERATION_REMOTE_COMPILATION_CACHE=default). The helpers are re-exported through runtime-core and runtime so the node plugin reaches them via __webpack_require__.federation.runtime, with a new Function fallback for older runtimes. FEDERATION_KEEP_COMPILATION_CACHE is removed. Adds sdk behaviour tests, node fallback and httpEvalStrategy conformance tests, and a memory benchmark harness. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…icitly during removal Introduce resolveRemoteRuntimeInstance with monotonic matching (registered name + buildVersion, entryGlobalName + buildVersion, then name-only levels only when no buildVersion is known), split removeRemote into single-purpose steps, release shares by the instance's own producer name, and warn only on ambiguous or version-mismatched lookups. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A direct eval is cached by V8 under the calling script. When the sdk runs
inside a remote entry, each new build's copy evaluating eval('require') left an
eval-cache entry that pinned that entry's source for the life of the process
(measured ~0.2 MB per forced refresh with new builds). Resolve builtins once
per process on a shared global, preferring process.getBuiltinModule.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…, not federation.runtime Remove the compile helpers from the runtime and runtime-core exports; they are Node execution infrastructure, not runtime APIs. The sdk gains compileRemoteCommonJsModule (policy + compile), which loadScriptNode and the node runtime plugin use directly. The plugin no longer looks the helpers up on __webpack_require__.federation.runtime and no longer carries a new Function fallback for older runtimes: it compiles through its own dependency. That also removes the silent fallback path where a remote bundled against an older runtime namespace compiled chunks with no cache policy. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: e90a4b3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 48 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e90a4b3df1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const scopeId = | ||
| (attrs && (attrs['name'] || attrs['globalName'])) || filename; | ||
|
|
||
| withSideEffectScope(scopeId, () => { |
There was a problem hiding this comment.
With a Node remote declared as type: 'module' or type: 'esm', only the CommonJS execution is scoped here; the ESM branch at packages/sdk/src/node.ts:346-353 calls module.evaluate() directly. A top-level setInterval or process.on in an ESM entry therefore has no bucket under attrs.name, so force re-registering that remote cannot dispose the effect. Wrap the ESM evaluation with the same scope ID.
Useful? React with 👍 / 👎.
| const script = new vm.Script( | ||
| `(function(exports, require, __dirname, __filename) {${content}\n})`, | ||
| { | ||
| const scopeId = path.basename(filename); |
There was a problem hiding this comment.
When the local-filesystem chunk path is used, this records registrations made by the chunk wrapper under a basename such as 123.js. Teardown only disposes remote.name and, optionally, entryGlobalName (packages/runtime-core/src/remote/index.ts:761-766), so force replacement cannot reach this bucket and leaves those timers or listeners active. Thread the remote name into this helper and use that same scope ID.
Useful? React with 👍 / 👎.
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
2 participants
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.
Description
Fixes #5031
Follows discussion in #4566 and architectural boundary proposal in #4566 (comment)
In long-running Node SSR hosts, re-registering or force-reloading remotes (
registerRemotes(..., { force: true })) tears down internal runtime bookkeeping, but remotes registering global side effects at module evaluation time (e.g.setInterval,setTimeout,process.on / once / addListener / prependListenerfrom logging, APM, or polling utilities) leak across generations. These active handles retain the previous generation's chunk code, resulting in linear memory growth per forced refresh.Solution
Side-Effect Scope Lifecycle (
@module-federation/sdk):withSideEffectScope(scopeId, fn)with an explicit LIFO execution stack andtry/finallysafety.setTimeout,setInterval,setImmediate, andprocess.on / once / addListener / prependListenerto attribute handles to the activescopeId.disposeRemoteSideEffects(scopeId).Wiring Points:
createScriptNodeevaluation in@module-federation/sdk/node.compileChunkinvocations inloadFromFsandfetchAndRunin@module-federation/node.wrapModuleFactoryin@module-federation/runtime-core.RemoteHandler.removeRemotecallsdisposeRemoteSideEffectswhendisposeSideEffects: trueorforce: trueis requested.Tests & Benchmarks:
@module-federation/sdk/__tests__/node-side-effects.spec.tscovering:processlisteners.@module-federation/runtime-coreand@module-federation/runtimeverifying side effects are cleared upon forced re-registration.packages/node/__benchmarks__/remote-side-effects-memory.mjs.