diff --git a/Jint.Tests.Test262/Test262Test.cs b/Jint.Tests.Test262/Test262Test.cs index d55fea1f0..01791ec0c 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 bf2b7f716..7d5ee914e 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 042c733d6..a77ed89f2 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 eaaa769f9..75b5c2acc 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 1d918b09c..88e6cb69b 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 8cc302b8b..98861ac61 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 7a6fea01b..eaa957721 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 8c40919bd..172a4a5ed 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 a8381073a..b8cc697ee 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);