Extend prepared-script engine-retention test to newer env cache shapes#2594
Merged
Conversation
Analysis of sebastienros#2587 confirmed that everything a prepared script retains after engine disposal is engine-independent prepare-time metadata by design (constant-folded primitives, FDI State, block declaration caches, binding names) and that no Engine/Realm/Environment can be reached from the shared AST. The regression test guarding that invariant predates two environment cache shapes added since: the for-of/for-in per-iteration environment cache on the JintForInForOfStatement handler (sebastienros#2586) and the Function-constructor definition-level environment parked on the realm-cached dynamic State (sebastienros#2579). Cover both so a future refactor moving either onto shared AST state fails the test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lahma
enabled auto-merge (squash)
July 6, 2026 21:27
8 tasks
This was referenced Jul 13, 2026
This was referenced Jul 20, 2026
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.
Follow-up to the analysis of #2587.
Analysis summary
Walking every object a prepared script can retain after engine disposal (everything parked on AST
UserDataat prepare/build time) confirms the v4.11.0 snapshot in #2587 is fully by design — nothing engine-owned survives:JintConstantExpression+JsString/JsNumberJintFunctionDefinition.State_dynamicCachedEnv) is gated to Function-constructor definitions living in the per-realm cache, never on prepared ASTs;_cachedSlotsis cleared before parkingJintBlockStatement.BlockStateDeclarationCache, 16→28 B)Environment.BindingNameJsStringper identifier nameCalculatedValueis readonly and only ever theUndefinedsingletonCachedHoistingScope,ConstantStatementThe net retained size actually dropped vs v4.4.0 (127.8 → 120.7 MB) because identifiers now park a single deduplicated
BindingNameper name instead of oneJintIdentifierExpressionhandler per node (was ~8 MB in the v4.4.0 snapshot).Change
GarbageCollectionTests.SharedPreparedScriptDoesNotRetainEngineslocks the invariant that a shared prepared script must not pin engines, but it predates two environment-cache shapes added since it was written:JintForInForOfStatementhandler (Reuse a fixed-slot per-iteration environment in for-of/for-in #2586)State(Definition-level environment reuse for Function-constructor instances #2579)This extends the test script to exercise both, so a future refactor moving either cache onto shared AST state fails the test. Verified green on net10.0 and net472.
No product code changes; #2587 needs no fix.
🤖 Generated with Claude Code