TimeoutInterval doesn't work for a custom comparer function of Array.sort that is ArrowFunctionExpression

[lekrus]

Hello,

In #809 Array.Sort was replaced to LINQ OrderBy
Since that change, some bad comparers (that don't fit requirements) provided to Array.sort function resulted in never-ending execution.

Sample:

let cases = [5,5];
let latestCase = cases.sort((c1, c2) => c1 > c2 ? -1: 1);

as soon as that expression will not return 0 for equal elements, that sort function is inconsistent.

The issue is stably reproducible only if the test solution is compiled for .NET Framework
For .NET Core, OrderBy uses different implementation and it doesn't result in never-ending execution.
But according to documentation to CompareTo that is still possible for specially crafted comparer functions.

We have defined TimeoutInterval constraint for executing Engine, but for the case described above that doesn't work.
Most probably due to using of arrow function expression.
For example, just embracing that in {} will result in TimeoutException when that issue occurs:

let cases = [5,5];
let latestCase = cases.sort((c1, c2) => {return c1 > c2 ? -1: 1;});

Is it possible to enhance current constraint implementation to work for ArrowFunctionExpression too?

Also, as soon as Jint knows the specific comparer and a first element of sorted array, is it possible to add a very lightweight check for consistency of the comparer? Like to check that FN(elem, elem) returns 0 and if that is not true - then throw some exception?
I know that is only a single specific case of inconsistency, but it's very lightweight check and could help investigating problem faster (not only by TimeoutExceptions)

Thanks!

[sebastienros]

What should be done is wrap the function call and increment the execution steps to ensure the limit is applied.
I don't think we can invoke the function beforehand since that would break the spec and unit tests, some comparers have state.

There might be other places where this would make sense.

[lahma]

@lekrus can you test with latest Jint 3.x beta? My test case works fine under full framework (on NET 5 it doesn't throw as there's the logic you've described protecting). Might have been fixed during some of the unification work done with function handling.

[Fact]
public void ThrowsTimeout()
{
    const string script = @"
                    let cases = [5,5];
                    let latestCase = cases.sort((c1, c2) => { return c1 > c2 ? -1: 1; });";

    var engine = new Engine(options => options.TimeoutInterval(TimeSpan.FromSeconds(2)));

    Assert.Throws<TimeoutException>(() => engine.Execute(script));
}

[lekrus]

Hello Marko, thanks for checking the issue.
That test actually works, but it was working before as well.

It still doesn't work when there is no return in that arrow function expression:

let cases = [5,5];
let latestCase = cases.sort((c1, c2) => c1 > c2 ? -1: 1);