diff --git a/Jint.Tests/Runtime/GarbageCollectionTests.cs b/Jint.Tests/Runtime/GarbageCollectionTests.cs new file mode 100644 index 0000000000..042c733d6d --- /dev/null +++ b/Jint.Tests/Runtime/GarbageCollectionTests.cs @@ -0,0 +1,65 @@ +namespace Jint.Tests.Runtime; + +// This needs to run without any parallelization because it uses +// garbage collector metrics which cannot be isolated. +[CollectionDefinition(nameof(GarbageCollectionTests), DisableParallelization = true)] +[Collection(nameof(GarbageCollectionTests))] +public class GarbageCollectionTests +{ + [Fact] + public void InternalCachingDoesNotPreventGarbageCollection() + { + // This test ensures that memory allocated within functions + // can be garbage collected by the .NET runtime. To test that, + // the "allocate" functions allocates a big chunk of memory, + // which is not used anywhere. So the GC should have no problem + // releasing that memory after the "allocate" function leaves. + + // Arrange + var engine = new Engine(); + const string script = + """ + function allocate(runAllocation) { + if (runAllocation) { + // Allocate ~200 MB of data (not 100 because .NET uses UTF-16 for strings) + var test = Array.from({ length: 100 }) + .map(() => ' '.repeat(1 * 1024 * 1024)); + } + return 2; + } + """; + engine.Evaluate(script); + + // Create a baseline for memory usage. + engine.Evaluate("allocate(false);"); + var usedMemoryBytesBaseline = CurrentlyUsedMemory(); + + // Act + engine.Evaluate("allocate(true);"); + + // Assert + var usedMemoryBytesAfterJsScript = CurrentlyUsedMemory(); + var epsilon = 10 * 1024 * 1024; // allowing up to 10 MB of other allocations should be enough to prevent false positives + Assert.True( + usedMemoryBytesAfterJsScript - usedMemoryBytesBaseline < epsilon, + userMessage: $""" + The garbage collector did not free the allocated but unreachable 200 MB from the script.; + Before Call : {BytesToString(usedMemoryBytesBaseline)} + After Call : {BytesToString(usedMemoryBytesAfterJsScript)} + --- + Acceptable : {BytesToString(usedMemoryBytesBaseline + epsilon)} + """); + return; + + static string BytesToString(long bytes) + => $"{(bytes / 1024.0 / 1024.0),6:0.0} MB"; + + static long CurrentlyUsedMemory() + { + // Just try to ensure that everything possible gets collected. + GC.Collect(2, GCCollectionMode.Forced, blocking: true); + var currentlyUsed = GC.GetTotalMemory(forceFullCollection: true); + return currentlyUsed; + } + } +} diff --git a/Jint/Collections/RefStack.cs b/Jint/Collections/RefStack.cs index c408b44bdc..94b9ab3ee3 100644 --- a/Jint/Collections/RefStack.cs +++ b/Jint/Collections/RefStack.cs @@ -49,8 +49,12 @@ public bool TryPeek([NotNullWhen(true)] out T item) return false; } + /// + /// Returns the last added item from the stack and decreases the stack size by one. + /// + /// Is thrown if the stack is empty. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ref readonly T Pop() + public T Pop() { if (_size == 0) { @@ -58,7 +62,34 @@ public ref readonly T Pop() } _size--; - return ref _array[_size]; + // Create a copy of the value because it will get overridden + // in the statement. + var result = _array[_size]; + // Ensure that the item can get garbage collected if possible. + _array[_size] = default; + return result; + } + + /// + /// Same as but without copying the value. + /// + /// + /// Use this method when the return value is not used and + /// is a value type. In that case, this methods + /// avoids one value type copy compared to . + /// + /// Is thrown if the stack is empty. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void PopAndDiscard() + { + if (_size == 0) + { + Throw.InvalidOperationException("stack is empty"); + } + + _size--; + // Ensure that the item can get garbage collected if possible. + _array[_size] = default; } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -113,7 +144,18 @@ private void Resize(int value) public void Clear() { - _size = 0; + if (_size > 0) + { +#if NETFRAMEWORK || NETSTANDARD2_0 + for (var i = 0; i < _size; i++) + { + _array[i] = default; + } +#else + Array.Fill(_array, default, 0, _size); +#endif + _size = 0; + } } public Enumerator GetEnumerator() diff --git a/Jint/Pooling/JsValueArrayPool.cs b/Jint/Pooling/JsValueArrayPool.cs index ca6a77fa6e..bd3b5ceee3 100644 --- a/Jint/Pooling/JsValueArrayPool.cs +++ b/Jint/Pooling/JsValueArrayPool.cs @@ -61,16 +61,26 @@ public JsValue[] RentArray(int size) [MethodImpl(MethodImplOptions.AggressiveInlining)] public void ReturnArray(JsValue[] array) { + // Ensure that the array contents are cleared + // to allow garbage collecting the values of the + // array if possible. Only the array itself + // should be cached. if (array.Length == 1) { + array[0] = null!; _poolArray1.Free(array); } else if (array.Length == 2) { + array[0] = null!; + array[1] = null!; _poolArray2.Free(array); } else if (array.Length == 3) { + array[0] = null!; + array[1] = null!; + array[2] = null!; _poolArray3.Free(array); } } diff --git a/Jint/Runtime/CallStack/JintCallStack.cs b/Jint/Runtime/CallStack/JintCallStack.cs index c7d4a2060c..fe3d7bd0d5 100644 --- a/Jint/Runtime/CallStack/JintCallStack.cs +++ b/Jint/Runtime/CallStack/JintCallStack.cs @@ -80,7 +80,7 @@ public int Push(Function function, JintExpression? expression, in ExecutionConte public CallStackElement Pop() { - ref readonly var item = ref _stack.Pop(); + var item = _stack.Pop(); if (_statistics is not null) { if (_statistics[item] == 0) diff --git a/Jint/Runtime/ExecutionContextStack.cs b/Jint/Runtime/ExecutionContextStack.cs index 597ab348dc..47b10d1925 100644 --- a/Jint/Runtime/ExecutionContextStack.cs +++ b/Jint/Runtime/ExecutionContextStack.cs @@ -58,7 +58,7 @@ public ref readonly ExecutionContext ReplaceTopGenerator(GeneratorInstance newEn public void Push(in ExecutionContext context) => _stack.Push(in context); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ref readonly ExecutionContext Pop() => ref _stack.Pop(); + public void Pop() => _stack.PopAndDiscard(); public IScriptOrModule? GetActiveScriptOrModule() { @@ -91,4 +91,4 @@ public ref readonly ExecutionContext ReplaceTopGenerator(GeneratorInstance newEn return null; } -} \ No newline at end of file +}