Issue with true 'parallel' async method - ReferenceError: Variable "param1" is not defined

[cyrildurand]

I encounter an error with async method on my production environment.

I'm not sure how to explain it but I could reproduce it with this minimal code sample

    [TestMethod]
    public void TestParallelAsync()
    {
        var script = Script.Parse(@"
export default async function(param1) {
    return param1?.value
}
");
        var context = new GlobalContext();
        var module = new Module($"main.js", script, context);
        module.Run();

        var run = module.Exports.Default.As<Function>();
        var tasks = Enumerable.Range(0, 10)
                              .Select(async i =>
                              {
                                  await Task.Yield();
                                  var result = run.Call(new Arguments() { null, new { value = "b" } });

                                  return await ((Promise)result.Value).Task;
                              });

        var task = Task.WhenAll(tasks);


        var values = task.GetAwaiter().GetResult();
        Assert.AreEqual("b", values[0].As<string>());
        Assert.AreEqual("b", values[1].As<string>());
    }

Without Task.Yield() we don't have any error.

Let me know if you need further information

[nilproject]

Sorry for the long response.
There are two problems:

1. Semantics of C# function Call(..) is not the same as JS call(..). First parameter is not this. So, param1 is always undefined in your code
2. JS functions are not threadsafe. So, behavior with multithreading execution is undefined. For this problem there are a few several solutions:
   2.1. You can enable debugging for this context

var context = new GlobalContext()
{
    Debugging = true,
};
context.DebuggerCallback += delegate { };

some loss of performance, but (almost) threadsafe execution.

2.2. Add eval() into function:

export default async function(param1) {
    eval()
    return param1?.value
}

significantly less, but also with performance penalty.

2.3. The best solution, i suppose

export default async function(param1) {
    await void 1;
    return param1?.value
}

smaller performance penalty.

2.4. Also you can parse script few more times, once per thread and get different copies of module with independent internal caches (this is the reason of thread unsafety). Make pool of them, for example.

[cyrildurand]

Hello,

Thanks for the explanation.

oops I use var result = run.Call(null, new Arguments() { new { value = "b" } }); not sure why the code I share was wrong.

        var tasks = Enumerable.Range(0, 10)
                              .Select(async i =>
                              {
                                  await Task.Yield();
                                  // await Task.CompletedTask; 
                                  var result = run.Call(null, new Arguments() { new { value = "b" } });

                                  return await ((Promise)result.Value).Task;
                              });

the test pass if I comment await Task.Yield();, I tried await Task.CompletedTask but the test still succeed.

I don't think the thread-safety explanation applies to my case, though. There is no multithreading involved in my application.
The Task.Yield() in the repro is only there to force an asynchronous continuation. After the await, execution resumes on a thread-pool thread because this is an MSTest test with no synchronization context, but there is never concurrent execution of the same JS context or module. Each invocation of run.Call(...) happens sequentially.

In production I also don't have multiple threads calling into the same GlobalContext concurrently, the code is asynchronous, but not parallel.

I tried

export default async function(param1) {
    await void 1;
    return param1?.value
}

but have the same issue.

with eval

export default async function(param1) {
    eval()
    return param1?.value
}

I don't have the issue

and with debugging, I don't have the issue.

BTW i tried to reproduce it in different way

using Microsoft.VisualStudio.TestTools.UnitTesting;
using NiL.JS;
using NiL.JS.BaseLibrary;
using NiL.JS.Core;
using NiL.JS.Extensions;
using System.Linq;
using System.Threading.Tasks;

namespace Tests.Fuzz;

[TestClass]
public sealed class Bug_317
{
    [TestMethod]
    public void Test1()
    {
        var script = Script.Parse(@"
export default async function(param1) {
    return param1?.value
}
");
        var context = new GlobalContext();
        var module = new Module($"main.js", script, context);
        module.Run();

        var run = module.Exports.Default.As<Function>();

        var tasks = Enumerable.Range(0, 10)
                              .Select(async i =>
                              {
                                  await Task.Yield();
                                  var result = run.Call(null, new Arguments() { new { value = "b" } });

                                  return await ((Promise)result.Value).Task;
                              });

        var task = Task.WhenAll(tasks);


        var values = task.GetAwaiter().GetResult();
        Assert.AreEqual("b", values[0].As<string>());
        Assert.AreEqual("b", values[1].As<string>());
    }

    [TestMethod]
    public async Task Test1bis()
    {
        var script = Script.Parse(@"
export default async function(param1) {
    return param1?.value
}
");
        var context = new GlobalContext();
        var module = new Module($"main.js", script, context);
        module.Run();

        var run = module.Exports.Default.As<Function>();

        var tasks = Enumerable.Range(0, 10)
                              .Select(async i =>
                              {
                                  await Task.Yield();
                                  var result = run.Call(null, new Arguments() { new { value = "b" } });

                                  return await ((Promise)result.Value).Task;
                              });

        var values = await Task.WhenAll(tasks);


        Assert.AreEqual("b", values[0].As<string>());
        Assert.AreEqual("b", values[1].As<string>());
    }



    [TestMethod]
    public async Task Test1WithAwaitInJs()
    {
        var script = Script.Parse(@"
export default async function(param1) {
    await void 1;
    return param1?.value
}
");
        var context = new GlobalContext();
        var module = new Module($"main.js", script, context);
        module.Run();

        var run = module.Exports.Default.As<Function>();

        var tasks = Enumerable.Range(0, 10)
                              .Select(async i =>
                              {
                                  await Task.Yield();
                                  var result = run.Call(null, new Arguments() { new { value = "b" } });

                                  return await ((Promise)result.Value).Task;
                              });

        var task = Task.WhenAll(tasks);


        var values = task.GetAwaiter().GetResult();
        Assert.AreEqual("b", values[0].As<string>());
        Assert.AreEqual("b", values[1].As<string>());
    }

    [TestMethod]
    public async Task Test1WithEval()
    {
        var script = Script.Parse(@"
export default async function(param1) {
    eval()
    return param1?.value
}
");
        var context = new GlobalContext();
        var module = new Module($"main.js", script, context);
        module.Run();

        var run = module.Exports.Default.As<Function>();

        var tasks = Enumerable.Range(0, 10)
                              .Select(async i =>
                              {
                                  await Task.Yield();
                                  var result = run.Call(null, new Arguments() { new { value = "b" } });

                                  return await ((Promise)result.Value).Task;
                              });

        var task = Task.WhenAll(tasks);


        var values = task.GetAwaiter().GetResult();
        Assert.AreEqual("b", values[0].As<string>());
        Assert.AreEqual("b", values[1].As<string>());
    }

    [TestMethod]
    public async Task Test1WithDebug()
    {
        var script = Script.Parse(@"
export default async function(param1) {
    return param1?.value
}
");
        var context = new GlobalContext()
        {
            Debugging = true
        };
        context.DebuggerCallback += delegate { };
        var module = new Module($"main.js", script, context);
        module.Run();

        var run = module.Exports.Default.As<Function>();

        var tasks = Enumerable.Range(0, 10)
                              .Select(async i =>
                              {
                                  await Task.Yield();
                                  var result = run.Call(null, new Arguments() { new { value = "b" } });

                                  return await ((Promise)result.Value).Task;
                              });

        var task = Task.WhenAll(tasks);


        var values = task.GetAwaiter().GetResult();
        Assert.AreEqual("b", values[0].As<string>());
        Assert.AreEqual("b", values[1].As<string>());
    }


    [TestMethod]
    public async Task Test1NoYield()
    {
        var script = Script.Parse(@"
export default async function(param1) {
    return param1?.value
}
");
        var context = new GlobalContext();
        var module = new Module($"main.js", script, context);
        module.Run();

        var run = module.Exports.Default.As<Function>();

        var tasks = Enumerable.Range(0, 10)
                              .Select(async i =>
                              {
                                  //await Task.Yield();
                                  var result = run.Call(null, new Arguments() { new { value = "b" } });

                                  return await ((Promise)result.Value).Task;
                              });

        var task = Task.WhenAll(tasks);


        var values = task.GetAwaiter().GetResult();
        Assert.AreEqual("b", values[0].As<string>());
        Assert.AreEqual("b", values[1].As<string>());
    }


    [TestMethod]
    public async Task Test2()
    {
        var script = Script.Parse(@"
export default async function(param1) {
    return param1?.value
}
");
        var context = new GlobalContext();
        var module = new Module($"main.js", script, context);
        module.Run();

        var run = module.Exports.Default.As<Function>();

        for (int i = 0; i < 10; i++)
        {
            await Task.Yield();

            var result = run.Call(null, new Arguments { new { value = "b" } });
            var value = await ((Promise)result.Value).Task;
            Assert.AreEqual("b", value.As<string>());
        }
    }


    [TestMethod]
    public async Task Test3()
    {
        var script = Script.Parse(@"
export default async function(param1) {
    return param1?.value
}
");
        var context = new GlobalContext();
        var module = new Module($"main.js", script, context);
        module.Run();

        var run = module.Exports.Default.As<Function>();

        for (int i = 0; i < 10; i++)
        {
            await Task.Run(async () =>
            {
                await Task.Yield();

                var result = run.Call(null, new Arguments { new { value = "b" } });
                var value = await ((Promise)result.Value).Task;
                Assert.AreEqual("b", value.As<string>());
            });
        }
    }
}

enabling debug could solve my issue but looks like a hack. would it be possible to support async operation ?

[cyrildurand]

I ask github copilot to create PR about this issue, 2 changes were introduced : #319 I'm not sure how it help.
let me know if you need more information

[nilproject]

await schedules continuation of method body as a task. Every continuation evaluates as a independent task by one of a thread, managed by task scheduler. Therefore there is multithreading execution.
Fix by Copilot is not complete, there are a few similar places in the codebase. I can't accept this fix.
It's ok to use one GlobalContext for all invocations, but not the module.

[cyrildurand]

I just let copilot work, I didn't have a look at the source code change.
would you like me to investigate more ? I could try to find more time to read the code