sebastienros / jint

Javascript Interpreter for .NET
BSD 2-Clause "Simplified" License
4.13k stars 560 forks source link

State of the instance of `Esprima.Ast.Program` class is changed by engine #667

Closed Taritsyn closed 5 years ago

Taritsyn commented 5 years ago

Hello!

In my project I use the pre-compilation of scripts to improve performance. When I used the Jint version 2.11.58, I could easily initialize several engines by one parsed script. To make it clear to you how this works, I will give a slightly artificial example:

using System;
using System.Linq;
using System.Threading.Tasks;

using Jint;
using Jint.Parser;

namespace TestJint2
{
    class Program
    {
        static void Main(string[] args)
        {
            const string libraryCode = @"function declensionOfNumerals(number, titles) {
    var result,
        titleIndex,
        cases = [2, 0, 1, 1, 1, 2],
        caseIndex
        ;

    if (number % 100 > 4 && number % 100 < 20) {
        titleIndex = 2;
    }
    else {
        caseIndex = number % 10 < 5 ? number % 10 : 5;
        titleIndex = cases[caseIndex];
    }

    result = titles[titleIndex];

    return result;
}

function declinationOfSeconds(number) {
    return declensionOfNumerals(number, ['секунда', 'секунды', 'секунд']);
}";
            const string functionName = "declinationOfSeconds";
            const int itemCount = 4;

            int[] inputSeconds = new int[itemCount] { 0, 1, 42, 600 };
            string[] targetOutputStrings = new string[itemCount] { "секунд", "секунда", "секунды", "секунд" };
            string[] outputStrings = new string[itemCount];

            var parserOptions = new ParserOptions()
            {
                Source = "declination-of-seconds.js"
            };
            var parser = new JavaScriptParser();
            var program = parser.Parse(libraryCode, parserOptions);

            var mainEngine = new Engine();
            mainEngine.Execute(program);
            outputStrings[0] = mainEngine.Invoke(functionName, inputSeconds[0]).AsString();

            Parallel.For(1, itemCount, itemIndex =>
            {
                var engine = new Engine();
                engine.Execute(program);
                outputStrings[itemIndex] = engine.Invoke(functionName, inputSeconds[itemIndex]).AsString();
            });

            if (outputStrings.SequenceEqual(targetOutputStrings))
            {
                Console.ForegroundColor = ConsoleColor.Green;
                Console.WriteLine("Success!");
            }
            else
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("Failed!");
                Console.WriteLine();

                for (int itemIndex = 0; itemIndex < itemCount; itemIndex++)
                {
                    string output = outputStrings[itemIndex];
                    string targetOutput = targetOutputStrings[itemIndex];
                    ConsoleColor color = output != targetOutput ? ConsoleColor.Red : ConsoleColor.White;

                    Console.ForegroundColor = color;
                    Console.WriteLine("{0}\t\t{1}", targetOutput, output);
                }
            }

            Console.ForegroundColor = ConsoleColor.White;
            Console.WriteLine();
        }
    }
}

But after updating the Jint to version 3.0.0 Beta 1598 (also tested on version 14033f2), a similar example began to return unpredictable results:

using System;
using System.Linq;
using System.Threading.Tasks;

using Esprima;
using Jint;

namespace TestJint3
{
    class Program
    {
        static void Main(string[] args)
        {
            const string libraryCode = @"function declensionOfNumerals(number, titles) {
    var result,
        titleIndex,
        cases = [2, 0, 1, 1, 1, 2],
        caseIndex
        ;

    if (number % 100 > 4 && number % 100 < 20) {
        titleIndex = 2;
    }
    else {
        caseIndex = number % 10 < 5 ? number % 10 : 5;
        titleIndex = cases[caseIndex];
    }

    result = titles[titleIndex];

    return result;
}

function declinationOfSeconds(number) {
    return declensionOfNumerals(number, ['секунда', 'секунды', 'секунд']);
}";
            const string functionName = "declinationOfSeconds";
            const int itemCount = 4;

            int[] inputSeconds = new int[itemCount] { 0, 1, 42, 600 };
            string[] targetOutputStrings = new string[itemCount] { "секунд", "секунда", "секунды", "секунд" };
            string[] outputStrings = new string[itemCount];

            var parserOptions = new ParserOptions("declination-of-seconds.js")
            {
                AdaptRegexp = true,
                Tolerant = true,
                Loc = true
            };
            var parser = new JavaScriptParser(libraryCode, parserOptions);
            var program = parser.ParseProgram();

            var mainEngine = new Engine();
            mainEngine.Execute(program);
            outputStrings[0] = mainEngine.Invoke(functionName, inputSeconds[0]).AsString();

            Parallel.For(1, itemCount, itemIndex =>
            {
                var engine = new Engine();
                engine.Execute(program);
                outputStrings[itemIndex] = engine.Invoke(functionName, inputSeconds[itemIndex]).AsString();
            });

            if (outputStrings.SequenceEqual(targetOutputStrings))
            {
                Console.ForegroundColor = ConsoleColor.Green;
                Console.WriteLine("Success!");
            }
            else
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("Failed!");
                Console.WriteLine();

                for (int itemIndex = 0; itemIndex < itemCount; itemIndex++)
                {
                    string output = outputStrings[itemIndex];
                    string targetOutput = targetOutputStrings[itemIndex];
                    ConsoleColor color = output != targetOutput ? ConsoleColor.Red : ConsoleColor.White;

                    Console.ForegroundColor = color;
                    Console.WriteLine("{0}\t\t{1}", targetOutput, output);
                }
            }

            Console.ForegroundColor = ConsoleColor.White;
            Console.WriteLine();
        }
    }
}

The first call always returns the correct result, and results of remaining three calls, that are made in parallel, are completely unpredictable. Most likely, this is due to the fact that the engine is now changing the state of instance of Esprima.Ast.Program class during execution. I was able to temporarily solve this problem by deep copying of parsed scripts (I use the extension method from the .NET Object Deep Copy project).

But to solve this problem, need more suitable solution, which will allow maintaining high performance. There are two variants:

  1. Return to the approach from second version, when the state of parsed script was immutable.
  2. Implement an ICloneable interface in the Esprima.Ast.Program class to make deep copy of the instance in most optimal way.
sebastienros commented 5 years ago

I think I found the reason. For the Literal AST node we cache the .NET value in the JintLiteralExpression one of Jint. But for the CallExpression (JintCallExpression) we rely on the AST to hold the value. And if you look at the JintCallExpression class you'll see it's not thread-safe in this case. This is the issue I think.

Would you be able to change this class locally to either store the cached value in the Jint expression itself, or to add some locking on the node when caching is done?

lahma commented 5 years ago

Created https://github.com/sebastienros/jint/pull/668 to address this issue, going to do separate PR for Esprima to remove the state from there.

Taritsyn commented 5 years ago

Thanks!