Fix race condition in sharing of function state (#1465)

sebastienros · Feb 23, 2023 · 68f6b5b · 68f6b5b
1 parent 8a03358
commit 68f6b5b
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 65 deletions.
diff --git a/Jint.Tests.CommonScripts/ConcurrencyTest.cs b/Jint.Tests.CommonScripts/ConcurrencyTest.cs
@@ -0,0 +1,25 @@
+using Esprima;
+
+namespace Jint.Tests.CommonScripts;
+
+[Parallelizable(ParallelScope.Fixtures)]
+public class ConcurrencyTest
+{
+    [Test]
+    [TestCase(true)]
+    [TestCase(false)]
+    public void ConcurrentEnginesCanUseSameAst(bool prepared)
+    {
+        var scriptContents = SunSpiderTests.GetEmbeddedFile("babel-standalone.js");
+        var script = prepared
+            ? Engine.PrepareScript(scriptContents)
+            : new JavaScriptParser().ParseScript(scriptContents);
+
+        Parallel.ForEach(Enumerable.Range(0, 3), x =>
+        {
+            new Engine()
+                .SetValue("assert", new Action<bool, string>((condition, message)=> { }))
+                .Evaluate(script);
+        });
+    }
+}
diff --git a/Jint.Tests.CommonScripts/SunSpiderTests.cs b/Jint.Tests.CommonScripts/SunSpiderTests.cs
@@ -1,73 +1,72 @@
 using System.Reflection;
 using Jint.Runtime;
 
-namespace Jint.Tests.CommonScripts
+namespace Jint.Tests.CommonScripts;
+
+[Parallelizable(ParallelScope.All)]
+public class SunSpiderTests
 {
-    public class SunSpiderTests
+    private static void RunTest(string source)
     {
-        private static void RunTest(string source)
-        {
-            var engine = new Engine()
-                    .SetValue("log", new Action<object>(Console.WriteLine))
-                    .SetValue("assert", new Action<bool, string>((condition, message) => Assert.True(condition, message)));
+        var engine = new Engine()
+            .SetValue("log", new Action<object>(Console.WriteLine))
+            .SetValue("assert", new Action<bool, string>((condition, message) => Assert.True(condition, message)));
 
-            try
-            {
-                engine.Execute(source);
-            }
-            catch (JavaScriptException je)
-            {
-                throw new Exception(je.ToString());
-            }
+        try
+        {
+            engine.Execute(source);
         }
+        catch (JavaScriptException je)
+        {
+            throw new Exception(je.ToString());
+        }
+    }
 
-        [Test]
-        [Parallelizable(ParallelScope.All)]
-        [TestCase("3d-cube.js")]
-        [TestCase("3d-morph.js")]
-        [TestCase("3d-raytrace.js")]
-        [TestCase("access-binary-trees.js")]
-        [TestCase("access-fannkuch.js")]
-        [TestCase("access-nbody.js")]
-        [TestCase("access-nsieve.js")]
-        [TestCase("bitops-3bit-bits-in-byte.js")]
-        [TestCase("bitops-bits-in-byte.js")]
-        [TestCase("bitops-bitwise-and.js")]
-        [TestCase("bitops-nsieve-bits.js")]
+    [Test]
+    [TestCase("3d-cube.js")]
+    [TestCase("3d-morph.js")]
+    [TestCase("3d-raytrace.js")]
+    [TestCase("access-binary-trees.js")]
+    [TestCase("access-fannkuch.js")]
+    [TestCase("access-nbody.js")]
+    [TestCase("access-nsieve.js")]
+    [TestCase("bitops-3bit-bits-in-byte.js")]
+    [TestCase("bitops-bits-in-byte.js")]
+    [TestCase("bitops-bitwise-and.js")]
+    [TestCase("bitops-nsieve-bits.js")]
 #if !DEBUG // should only be ran in release mode when inlining happens
-        [TestCase("controlflow-recursive.js")]
+    [TestCase("controlflow-recursive.js")]
 #endif
-        [TestCase("crypto-aes.js")]
-        [TestCase("crypto-md5.js")]
-        [TestCase("crypto-sha1.js")]
-        [TestCase("date-format-tofte.js")]
-        [TestCase("date-format-xparb.js")]
-        [TestCase("math-cordic.js")]
-        [TestCase("math-partial-sums.js")]
-        [TestCase("math-spectral-norm.js")]
-        [TestCase("regexp-dna.js")]
-        [TestCase("string-base64.js")]
-        [TestCase("string-fasta.js")]
-        [TestCase("string-tagcloud.js")]
-        [TestCase("string-unpack-code.js")]
-        [TestCase("string-validate-input.js")]
-        [TestCase("babel-standalone.js")]
-        public void Sunspider(string url)
-        {
-            var content = GetEmbeddedFile(url);
-            RunTest(content);
-        }
+    [TestCase("crypto-aes.js")]
+    [TestCase("crypto-md5.js")]
+    [TestCase("crypto-sha1.js")]
+    [TestCase("date-format-tofte.js")]
+    [TestCase("date-format-xparb.js")]
+    [TestCase("math-cordic.js")]
+    [TestCase("math-partial-sums.js")]
+    [TestCase("math-spectral-norm.js")]
+    [TestCase("regexp-dna.js")]
+    [TestCase("string-base64.js")]
+    [TestCase("string-fasta.js")]
+    [TestCase("string-tagcloud.js")]
+    [TestCase("string-unpack-code.js")]
+    [TestCase("string-validate-input.js")]
+    [TestCase("babel-standalone.js")]
+    public void Sunspider(string url)
+    {
+        var content = GetEmbeddedFile(url);
+        RunTest(content);
+    }
 
-        private string GetEmbeddedFile(string filename)
-        {
-            const string prefix = "Jint.Tests.CommonScripts.Scripts.";
+    internal static string GetEmbeddedFile(string filename)
+    {
+        const string Prefix = "Jint.Tests.CommonScripts.Scripts.";
 
-            var assembly = typeof(SunSpiderTests).GetTypeInfo().Assembly;
-            var scriptPath = prefix + filename;
+        var assembly = typeof(SunSpiderTests).GetTypeInfo().Assembly;
+        var scriptPath = Prefix + filename;
 
-            using var stream = assembly.GetManifestResourceStream(scriptPath);
-            using var sr = new StreamReader(stream);
-            return sr.ReadToEnd();
-        }
+        using var stream = assembly.GetManifestResourceStream(scriptPath);
+        using var sr = new StreamReader(stream);
+        return sr.ReadToEnd();
     }
 }
diff --git a/Jint/Engine.cs b/Jint/Engine.cs
@@ -1064,8 +1064,9 @@ internal JsValue ResolveThisBinding()
                 var realm = Realm;
                 foreach (var f in configuration.FunctionsToInitialize)
                 {
-                    var fn = f.Name!;
-                    var fo = realm.Intrinsics.Function.InstantiateFunctionObject(f, lexEnv, privateEnv);
+                    var jintFunctionDefinition = new JintFunctionDefinition(f);
+                    var fn = jintFunctionDefinition.Name!;
+                    var fo = realm.Intrinsics.Function.InstantiateFunctionObject(jintFunctionDefinition, lexEnv, privateEnv);
                     varEnv.SetMutableBinding(fn, fo, strict: false);
                 }
             }

diff --git a/Jint/Runtime/Interpreter/Expressions/JintBinaryExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintBinaryExpression.cs
@@ -161,7 +161,7 @@ internal static JintExpression Build(BinaryExpression expression)
                     try
                     {
                         var context = new EvaluationContext();
-                        return new JintConstantExpression(expression, result.GetValue(context));
+                        return new JintConstantExpression(expression, (JsValue) result.EvaluateWithoutNodeTracking(context));
                     }
                     catch
                     {

diff --git a/Jint/Runtime/Interpreter/Expressions/JintExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintExpression.cs
@@ -56,6 +56,18 @@ public object Evaluate(EvaluationContext context)
             return result;
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal object EvaluateWithoutNodeTracking(EvaluationContext context)
+        {
+            if (!_initialized)
+            {
+                Initialize(context);
+                _initialized = true;
+            }
+
+            return EvaluateInternal(context);
+        }
+
         /// <summary>
         /// Opportunity to build one-time structures and caching based on lexical context.
         /// </summary>

diff --git a/Jint/Runtime/Interpreter/JintFunctionDefinition.cs b/Jint/Runtime/Interpreter/JintFunctionDefinition.cs
@@ -170,7 +170,7 @@ internal sealed class State
         public bool HasParameterExpressions;
         public bool ArgumentsObjectNeeded;
         public List<Key>? VarNames;
-        public LinkedList<JintFunctionDefinition>? FunctionsToInitialize;
+        public LinkedList<FunctionDeclaration>? FunctionsToInitialize;
         public readonly HashSet<Key> FunctionNames = new();
         public LexicalVariableDeclaration[] LexicalDeclarations = Array.Empty<LexicalVariableDeclaration>();
         public HashSet<Key>? ParameterBindings;
@@ -200,18 +200,18 @@ internal static State BuildState(IFunction function)
         var lexicalNames = hoistingScope._lexicalNames;
         state.VarNames = hoistingScope._varNames;
 
-        LinkedList<JintFunctionDefinition>? functionsToInitialize = null;
+        LinkedList<FunctionDeclaration>? functionsToInitialize = null;
 
         if (functionDeclarations != null)
         {
-            functionsToInitialize = new LinkedList<JintFunctionDefinition>();
+            functionsToInitialize = new LinkedList<FunctionDeclaration>();
             for (var i = functionDeclarations.Count - 1; i >= 0; i--)
             {
                 var d = functionDeclarations[i];
                 var fn = d.Id!.Name;
                 if (state.FunctionNames.Add(fn))
                 {
-                    functionsToInitialize.AddFirst(new JintFunctionDefinition(d));
+                    functionsToInitialize.AddFirst(d);
                 }
             }
         }