Browse Source

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

Marko Lahma 2 years ago
parent
commit
68f6b5bf93

+ 25 - 0
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);
+        });
+    }
+}

+ 57 - 58
Jint.Tests.CommonScripts/SunSpiderTests.cs

@@ -1,73 +1,72 @@
 using System.Reflection;
 using System.Reflection;
 using Jint.Runtime;
 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
 #if !DEBUG // should only be ran in release mode when inlining happens
-        [TestCase("controlflow-recursive.js")]
+    [TestCase("controlflow-recursive.js")]
 #endif
 #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();
     }
     }
 }
 }

+ 3 - 2
Jint/Engine.cs

@@ -1064,8 +1064,9 @@ namespace Jint
                 var realm = Realm;
                 var realm = Realm;
                 foreach (var f in configuration.FunctionsToInitialize)
                 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);
                     varEnv.SetMutableBinding(fn, fo, strict: false);
                 }
                 }
             }
             }

+ 1 - 1
Jint/Runtime/Interpreter/Expressions/JintBinaryExpression.cs

@@ -161,7 +161,7 @@ namespace Jint.Runtime.Interpreter.Expressions
                     try
                     try
                     {
                     {
                         var context = new EvaluationContext();
                         var context = new EvaluationContext();
-                        return new JintConstantExpression(expression, result.GetValue(context));
+                        return new JintConstantExpression(expression, (JsValue) result.EvaluateWithoutNodeTracking(context));
                     }
                     }
                     catch
                     catch
                     {
                     {

+ 12 - 0
Jint/Runtime/Interpreter/Expressions/JintExpression.cs

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

+ 4 - 4
Jint/Runtime/Interpreter/JintFunctionDefinition.cs

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