From df4a06e2bea3d4835f535f7840bfc38134826796 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Thu, 2 Jul 2026 16:48:49 +0300 Subject: [PATCH] Make Function.prototype.toString() source text retention opt-in (#2560) Since v4.9.0 (#2394) every parsed function node pins the entire source string (Node.UserData = ctx.Input) so Function.prototype.toString() can return the original source, released only lazily on the first toString() call. Applications that cache many large prepared scripts and never call toString() therefore retain hundreds of MB of duplicated source strings (issue #2560: ~250 MB, the same ~500 KB string per engine). Add an opt-in RetainFunctionSourceText parsing option (default false) plus an engine-level Options.RetainFunctionSourceText toggle. When disabled (the default) the source text is not retained and toString() returns the "function name() { [native code] }" placeholder. - RetainFunctionSourceText on IParsingOptions + ScriptParsingOptions/ModuleParsingOptions - ApplyTo wires OnNode conditionally; prepared path passes null/ctx.Input in AstAnalyzer - Options.RetainFunctionSourceText + fluent extension; engine default parser honors it - Test262 harness and the #2394 source-asserting tests opt in to keep conformance green - Retained-memory regression test: 25 cached ~800 KB scripts retain 20.1 MB vs 1.8 MB Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9 --- Jint.Tests.Test262/Test262Test.cs | 9 ++- Jint.Tests/Runtime/FunctionTests.cs | 41 +++++++++- Jint.Tests/Runtime/GarbageCollectionTests.cs | 79 +++++++++++++++++--- Jint/Engine.Ast.cs | 4 +- Jint/Engine.cs | 8 +- Jint/Options.Extensions.cs | 11 +++ Jint/Options.cs | 13 ++++ Jint/ParsingOptions.cs | 34 ++++++++- Jint/Test262Object.cs | 2 +- 9 files changed, 182 insertions(+), 19 deletions(-) diff --git a/Jint.Tests.Test262/Test262Test.cs b/Jint.Tests.Test262/Test262Test.cs index d55fea1f05..01791ec0c8 100644 --- a/Jint.Tests.Test262/Test262Test.cs +++ b/Jint.Tests.Test262/Test262Test.cs @@ -138,14 +138,19 @@ private static void ExecuteTest(Engine engine, Test262File file) if (file.Type == ProgramType.Module) { var specifier = "./" + Path.GetFileName(file.FileName); - engine.Modules.Add(specifier, builder => builder.AddSource(file.Program)); + engine.Modules.Add(specifier, builder => builder + .WithOptions(static options => options with { RetainFunctionSourceText = true }) + .AddSource(file.Program)); engine.Modules.Import(specifier); } else { var script = Engine.PrepareScript(file.Program, source: file.FileName, options: new ScriptPreparationOptions { - ParsingOptions = ScriptParsingOptions.Default with { Tolerant = false, AllowReturnOutsideFunction = false }, + // Retain source text so Function.prototype.toString() conformance tests + // (built-ins/Function/prototype/toString/*) validate the real-source output, which is + // opt-in since the engine no longer retains source by default (issue #2560). + ParsingOptions = ScriptParsingOptions.Default with { Tolerant = false, AllowReturnOutsideFunction = false, RetainFunctionSourceText = true }, }); engine.Execute(script); diff --git a/Jint.Tests/Runtime/FunctionTests.cs b/Jint.Tests/Runtime/FunctionTests.cs index bf2b7f7167..7d5ee914e9 100644 --- a/Jint.Tests/Runtime/FunctionTests.cs +++ b/Jint.Tests/Runtime/FunctionTests.cs @@ -772,12 +772,12 @@ function fn() { ( )=>0 """)] - public void ToStringShouldProduceStandardsCompliantResultByDefault(string code, string expectedResult) + public void ToStringShouldProduceStandardsCompliantResultWhenSourceTextRetained(string code, string expectedResult) { code = Regex.Replace(code, @"\r\n?", "\n", RegexOptions.CultureInvariant); // normalize line endings to '\n' expectedResult = Regex.Replace(expectedResult, @"\r\n?", "\n", RegexOptions.CultureInvariant); // normalize line endings to '\n' - var returnValue = new Engine().Evaluate(code); + var returnValue = new Engine(options => options.RetainFunctionSourceText()).Evaluate(code); var actualResults = Assert.IsType(returnValue); Assert.Equal(2u, actualResults.Length); @@ -788,4 +788,41 @@ public void ToStringShouldProduceStandardsCompliantResultByDefault(string code, var actualResult2 = Assert.IsType(actualResults[1]).ToString(); Assert.Same(actualResult1, actualResult2); } + + [Fact] + public void ToStringReturnsNativeCodePlaceholderByDefault() + { + // By default the source text is not retained (to save memory), so toString() returns the + // native-code placeholder rather than the original source. See issue #2560. + var engine = new Engine(); + + Assert.Equal("function fn() { [native code] }", engine.Evaluate("function fn() { return 0; } fn.toString();").AsString()); + Assert.Equal("function foo() { [native code] }", engine.Evaluate("var foo = function () {}; foo.toString();").AsString()); + Assert.Equal("function () { [native code] }", engine.Evaluate("(() => 0).toString();").AsString()); + } + + [Fact] + public void ToStringReturnsSourceTextWhenRetainedViaParsingOptions() + { + // Opt in to source-text retention through parsing options (covers the prepared-script path too). + var engine = new Engine(); + var parsingOptions = new ScriptParsingOptions { RetainFunctionSourceText = true }; + + Assert.Equal( + "function fn() { return 0; }", + engine.Evaluate("function fn() { return 0; } fn.toString();", parsingOptions).AsString()); + + var prepared = Engine.PrepareScript( + "function fn() { return 42; } fn.toString();", + options: new ScriptPreparationOptions { ParsingOptions = parsingOptions }); + Assert.Equal("function fn() { return 42; }", new Engine().Evaluate(prepared).AsString()); + } + + [Fact] + public void PreparedScriptDoesNotRetainSourceTextByDefault() + { + // A prepared script must not retain its source by default: toString() falls back to native code. + var prepared = Engine.PrepareScript("function fn() { return 0; } fn.toString();"); + Assert.Equal("function fn() { [native code] }", new Engine().Evaluate(prepared).AsString()); + } } diff --git a/Jint.Tests/Runtime/GarbageCollectionTests.cs b/Jint.Tests/Runtime/GarbageCollectionTests.cs index 042c733d6d..a77ed89f27 100644 --- a/Jint.Tests/Runtime/GarbageCollectionTests.cs +++ b/Jint.Tests/Runtime/GarbageCollectionTests.cs @@ -1,4 +1,7 @@ -namespace Jint.Tests.Runtime; +using System.Text; +using Acornima.Ast; + +namespace Jint.Tests.Runtime; // This needs to run without any parallelization because it uses // garbage collector metrics which cannot be isolated. @@ -49,17 +52,75 @@ function allocate(runAllocation) { --- Acceptable : {BytesToString(usedMemoryBytesBaseline + epsilon)} """); - return; + } + + [Fact] + public void PreparedScriptsDoNotRetainSourceTextByDefault() + { + // Regression test for #2560: by default a prepared script must not keep the full source text alive + // (it was retained per function node to back Function.prototype.toString()). With many large cached + // scripts this caused hundreds of MB of duplicated source strings. Holding N large scripts, the + // retaining variant must keep ~N * sourceSize more bytes alive than the non-retaining default. + // + // Each script is a tiny function wrapped around a large, unique comment: the source string is big + // (~commentChars * 2 bytes, UTF-16) while the AST is tiny, so the measured delta isolates the source. + + const int count = 25; + const int commentChars = 400_000; // ~800 KB of source per script + + var retained = MeasurePreparedHeap(count, commentChars, retainSourceText: true); + var notRetained = MeasurePreparedHeap(count, commentChars, retainSourceText: false); + + // Theoretical savings ≈ count * commentChars * 2 (UTF-16). Assert at least half to absorb noise. + var minimumSavings = (long) count * commentChars; // bytes; conservative lower bound (< chars * 2) + var actualSavings = retained - notRetained; + Assert.True( + actualSavings > minimumSavings, + userMessage: $""" + Disabling RetainFunctionSourceText did not free the expected source text. + Retained : {BytesToString(retained)} + Not retained : {BytesToString(notRetained)} + Savings : {BytesToString(actualSavings)} + Expected : > {BytesToString(minimumSavings)} + """); + + static long MeasurePreparedHeap(int count, int commentChars, bool retainSourceText) + { + var options = new ScriptPreparationOptions + { + ParsingOptions = new ScriptParsingOptions { RetainFunctionSourceText = retainSourceText }, + }; + + var prepared = new List>(count); + for (var i = 0; i < count; i++) + { + // The source string is built inline and never stored: only a retaining prepared script keeps + // it reachable. With retention off it becomes collectable once parsing completes. + prepared.Add(Engine.PrepareScript(BuildLargeScript(i, commentChars), options: options)); + } - static string BytesToString(long bytes) - => $"{(bytes / 1024.0 / 1024.0),6:0.0} MB"; + var used = CurrentlyUsedMemory(); + GC.KeepAlive(prepared); + return used; + } - static long CurrentlyUsedMemory() + static string BuildLargeScript(int seed, int commentChars) { - // Just try to ensure that everything possible gets collected. - GC.Collect(2, GCCollectionMode.Forced, blocking: true); - var currentlyUsed = GC.GetTotalMemory(forceFullCollection: true); - return currentlyUsed; + var sb = new StringBuilder(commentChars + 64); + sb.Append("function big").Append(seed).Append("() {\n /* ").Append(seed).Append(' '); + sb.Append('x', commentChars); // unique-ish, large comment body kept only in the source text + sb.Append(" */\n return 0;\n}\n"); + return sb.ToString(); } } + + private static long CurrentlyUsedMemory() + { + // Just try to ensure that everything possible gets collected. + GC.Collect(2, GCCollectionMode.Forced, blocking: true); + return GC.GetTotalMemory(forceFullCollection: true); + } + + private static string BytesToString(long bytes) + => $"{(bytes / 1024.0 / 1024.0),6:0.0} MB"; } diff --git a/Jint/Engine.Ast.cs b/Jint/Engine.Ast.cs index eaaa769f9d..75b5c2acc0 100644 --- a/Jint/Engine.Ast.cs +++ b/Jint/Engine.Ast.cs @@ -70,11 +70,13 @@ public static Prepared PrepareModule(string code, string? source = null, private sealed class AstAnalyzer { private readonly IPreparationOptions _preparationOptions; + private readonly bool _retainSourceText; private readonly Dictionary _bindingNames = new(StringComparer.Ordinal); public AstAnalyzer(IPreparationOptions preparationOptions) { _preparationOptions = preparationOptions; + _retainSourceText = preparationOptions.ParsingOptions.RetainFunctionSourceText; } public void NodeVisitor(Node node, in OnNodeContext ctx) @@ -113,7 +115,7 @@ public void NodeVisitor(Node node, in OnNodeContext ctx) case NodeType.ArrowFunctionExpression: case NodeType.FunctionDeclaration: case NodeType.FunctionExpression: - node.UserData = JintFunctionDefinition.BuildState((IFunction) node, ctx.Input); + node.UserData = JintFunctionDefinition.BuildState((IFunction) node, _retainSourceText ? ctx.Input : null); break; case NodeType.Program: diff --git a/Jint/Engine.cs b/Jint/Engine.cs index 1d918b09c6..88e6cb69b3 100644 --- a/Jint/Engine.cs +++ b/Jint/Engine.cs @@ -182,7 +182,10 @@ private Engine(Options? options, Action? configure) CallStack = new JintCallStack(Options.Constraints.MaxRecursionDepth >= 0); _stackGuard = new StackGuard(this); - var defaultParserOptions = ScriptParsingOptions.Default.GetParserOptions(Options); + var scriptParsingDefaults = Options.RetainFunctionSourceText + ? ScriptParsingOptions.RetainingDefault + : ScriptParsingOptions.Default; + var defaultParserOptions = scriptParsingDefaults.GetParserOptions(Options); _defaultParser = new Parser(defaultParserOptions); } @@ -229,7 +232,8 @@ internal Options Options public DebugHandler Debugger => _debugger ??= new DebugHandler(this, Options.Debugger.InitialStepMode); - internal ParserOptions DefaultModuleParserOptions => _defaultModuleParserOptions ??= ModuleParsingOptions.Default.GetParserOptions(Options); + internal ParserOptions DefaultModuleParserOptions => _defaultModuleParserOptions ??= + (Options.RetainFunctionSourceText ? ModuleParsingOptions.RetainingDefault : ModuleParsingOptions.Default).GetParserOptions(Options); internal ParserOptions GetActiveParserOptions() { diff --git a/Jint/Options.Extensions.cs b/Jint/Options.Extensions.cs index 8cc302b8bf..98861ac618 100644 --- a/Jint/Options.Extensions.cs +++ b/Jint/Options.Extensions.cs @@ -23,6 +23,17 @@ public static Options Strict(this Options options, bool strict = true) return options; } + /// + /// Whether the engine's default parser retains function source text so that + /// Function.prototype.toString() returns the original source. Defaults to + /// (returns a function name() { [native code] } placeholder and avoids keeping the source in memory). + /// + public static Options RetainFunctionSourceText(this Options options, bool retain = true) + { + options.RetainFunctionSourceText = retain; + return options; + } + /// /// Selects the handling for script debugger statements. /// diff --git a/Jint/Options.cs b/Jint/Options.cs index 7a6fea01b2..eaa9577218 100644 --- a/Jint/Options.cs +++ b/Jint/Options.cs @@ -78,6 +78,19 @@ public class Options /// public bool Strict { get; set; } + /// + /// Whether the engine's default parser retains the full source text of parsed functions so that + /// Function.prototype.toString() can return + /// the original source. Defaults to , in which case toString() returns a + /// function name() { [native code] } placeholder and the script source is not kept in memory. + /// + /// + /// This only affects scripts/modules parsed by the engine itself (e.g. 's Execute(string)). + /// When parsing via an explicit / or a + /// prepared script/module, the corresponding setting applies. + /// + public bool RetainFunctionSourceText { get; set; } + /// /// The culture the engine runs on, defaults to current culture. /// diff --git a/Jint/ParsingOptions.cs b/Jint/ParsingOptions.cs index 8c40919bdb..172a4a5edf 100644 --- a/Jint/ParsingOptions.cs +++ b/Jint/ParsingOptions.cs @@ -34,6 +34,16 @@ public interface IParsingOptions /// Defaults to . /// bool Tolerant { get; init; } + + /// + /// Gets or sets whether to retain the full source text of parsed functions so that + /// Function.prototype.toString() + /// can return the original source. + /// When (the default), the source text is not kept and toString() + /// returns a function name() { [native code] } placeholder. This avoids retaining the entire + /// script source in memory, which can be significant for large and/or cached (prepared) scripts. + /// + bool RetainFunctionSourceText { get; init; } } public sealed record ScriptParsingOptions : IParsingOptions @@ -43,11 +53,17 @@ public sealed record ScriptParsingOptions : IParsingOptions AllowReturnOutsideFunction = true, AllowTopLevelUsing = true, OnRegExp = Engine.DefaultConvertRegExpHandler, - OnNode = Engine.DefaultNodeHandler, + // OnNode (source-text retention) is applied conditionally in ApplyTo based on RetainFunctionSourceText. }; public static readonly ScriptParsingOptions Default = new(); + /// + /// A variant that retains function source text. Used by the engine when + /// is enabled to build its default parser. + /// + internal static readonly ScriptParsingOptions RetainingDefault = new() { RetainFunctionSourceText = true }; + /// /// Gets or sets whether to allow return statements at the top level. /// Defaults to . @@ -63,6 +79,9 @@ public sealed record ScriptParsingOptions : IParsingOptions /// public bool Tolerant { get; init; } = _defaultParserOptions.Tolerant; + /// + public bool RetainFunctionSourceText { get; init; } + /// /// Gets or sets the source location offset to apply when parsing. /// This allows mapping error locations back to the original source file @@ -76,6 +95,7 @@ internal ParserOptions ApplyTo(ParserOptions baseOptions, bool fallbackCompileRe { AllowReturnOutsideFunction = AllowReturnOutsideFunction, OnRegExp = GetOnRegExpHandler(fallbackCompileRegex, fallbackRegexTimeout), + OnNode = RetainFunctionSourceText ? Engine.DefaultNodeHandler : null, Tolerant = Tolerant, }; @@ -111,11 +131,17 @@ public sealed record class ModuleParsingOptions : IParsingOptions private static readonly ParserOptions _defaultParserOptions = Engine.BaseParserOptions with { OnRegExp = Engine.DefaultConvertRegExpHandler, - OnNode = Engine.DefaultNodeHandler, + // OnNode (source-text retention) is applied conditionally in ApplyTo based on RetainFunctionSourceText. }; public static readonly ModuleParsingOptions Default = new(); + /// + /// A variant that retains function source text. Used by the engine when + /// is enabled to build its default module parser. + /// + internal static readonly ModuleParsingOptions RetainingDefault = new() { RetainFunctionSourceText = true }; + /// public bool? CompileRegex { get; init; } @@ -125,9 +151,13 @@ public sealed record class ModuleParsingOptions : IParsingOptions /// public bool Tolerant { get; init; } = _defaultParserOptions.Tolerant; + /// + public bool RetainFunctionSourceText { get; init; } + internal ParserOptions ApplyTo(ParserOptions baseOptions, bool fallbackCompileRegex, TimeSpan fallbackRegexTimeout) => baseOptions with { OnRegExp = GetOnRegExpHandler(fallbackCompileRegex, fallbackRegexTimeout), + OnNode = RetainFunctionSourceText ? Engine.DefaultNodeHandler : null, Tolerant = Tolerant, }; diff --git a/Jint/Test262Object.cs b/Jint/Test262Object.cs index a8381073ad..b8cc697ee1 100644 --- a/Jint/Test262Object.cs +++ b/Jint/Test262Object.cs @@ -35,7 +35,7 @@ public static ObjectInstance Install(Engine engine) var script = Engine.PrepareScript(args.At(0).AsString(), options: new ScriptPreparationOptions { - ParsingOptions = ScriptParsingOptions.Default with { Tolerant = false }, + ParsingOptions = ScriptParsingOptions.Default with { Tolerant = false, RetainFunctionSourceText = true }, }); return engine.Evaluate(script);