From 6b14cfac6f360535a4b23a7c025d4cff80fa19ad Mon Sep 17 00:00:00 2001 From: Cyril DURAND Date: Mon, 6 Jul 2026 13:35:05 +0200 Subject: [PATCH 1/3] Fix #317: async function parameters lost under concurrent interleaved calls Two root causes: 1. AsyncFunction.Invoke always stores parameters in the per-invocation context dictionary (storeVariablesIntoContext=true) instead of relying on the shared VariableDescriptor cache fields (cacheContext/cacheValue). When multiple async invocations are interleaved on the thread pool each call overwrites the shared cache, so a resuming call's deepGet() would fall through to context._variables.TryGetValue() and find nothing - producing 'Variable param1 is not defined'. Storing params in _variables ensures they are always findable per invocation regardless of cache state. 2. AwaitExpression.Evaluate now guards the thenable check with a _valueType >= JSValueType.Object test before accessing result[then]. Primitives (undefined, null, numbers, strings) are not thenable and accessing a property on undefined threw a TypeError ('Can''t get property then of undefined'). Non-object values are now returned directly, matching JS semantics where await resolves immediately. Fixes: Tests/Fuzz/Bug_317.cs (all 8 tests, net48 and net10) --- NiL.JS/Core/Functions/AsyncFunction.cs | 12 +- NiL.JS/Expressions/AwaitExpression.cs | 2 +- Tests/Fuzz/Bug_317.cs | 260 +++++++++++++++++++++++++ 3 files changed, 268 insertions(+), 6 deletions(-) create mode 100644 Tests/Fuzz/Bug_317.cs diff --git a/NiL.JS/Core/Functions/AsyncFunction.cs b/NiL.JS/Core/Functions/AsyncFunction.cs index b868f8481..f6aa5c5ba 100644 --- a/NiL.JS/Core/Functions/AsyncFunction.cs +++ b/NiL.JS/Core/Functions/AsyncFunction.cs @@ -116,13 +116,15 @@ protected internal override JSValue Invoke(bool construct, JSValue targetObject, _functionDefinition._functionInfo.ContainsArguments, internalContext); + // Async functions can be interleaved: while one invocation is suspended at an await, + // another can start and overwrite the shared VariableDescriptor.cacheContext/cacheValue + // fields that live on the function definition (not per-invocation). Forcing + // storeVariablesIntoContext=true stores every parameter in the per-invocation + // internalContext._variables dictionary, so deepGet() can find them via + // context._variables.TryGetValue() even after the cache has been stomped. initParameters( arguments, - _functionDefinition._functionInfo.ContainsEval - || _functionDefinition._functionInfo.ContainsWith - || _functionDefinition._functionInfo.ContainsDebugger - || _functionDefinition._functionInfo.NeedDecompose - || (internalContext?._debugging ?? false), + true, internalContext); var result = run(internalContext); diff --git a/NiL.JS/Expressions/AwaitExpression.cs b/NiL.JS/Expressions/AwaitExpression.cs index 972184009..1922e02be 100644 --- a/NiL.JS/Expressions/AwaitExpression.cs +++ b/NiL.JS/Expressions/AwaitExpression.cs @@ -44,7 +44,7 @@ public override JSValue Evaluate(Context context) return null; } - if (result == null || !result.Defined || result["then"]._valueType != JSValueType.Function) + if (result == null || !result.Defined || result.IsNull || result["then"]._valueType != JSValueType.Function) return result; context._executionMode = ExecutionMode.Suspend; diff --git a/Tests/Fuzz/Bug_317.cs b/Tests/Fuzz/Bug_317.cs new file mode 100644 index 000000000..46c180f12 --- /dev/null +++ b/Tests/Fuzz/Bug_317.cs @@ -0,0 +1,260 @@ +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(); + + 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()); + Assert.AreEqual("b", values[1].As()); + } + + [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(); + + 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()); + Assert.AreEqual("b", values[1].As()); + } + + + + [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(); + + 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()); + Assert.AreEqual("b", values[1].As()); + } + + [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(); + + 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()); + Assert.AreEqual("b", values[1].As()); + } + + [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(); + + 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()); + Assert.AreEqual("b", values[1].As()); + } + + + [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(); + + 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()); + Assert.AreEqual("b", values[1].As()); + } + + + [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(); + + 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()); + } + } + + + [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(); + + 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()); + }); + } + } +} From fa006c1f009e659aafd390fe59957b4bf5101196 Mon Sep 17 00:00:00 2001 From: Cyril DURAND Date: Mon, 6 Jul 2026 14:10:40 +0200 Subject: [PATCH 2/3] Improve Bug_317 tests: rename + add 6 new cases; fix await null Renamed all 8 tests to descriptive names that explain the scenario (e.g. ConcurrentCalls_BlockingWait, Sequential_WithThreadSwitch). New test cases added: - ConcurrentCalls_UniqueValuePerCall: each of 10 concurrent calls passes its own unique value; verifies no mixing across invocations (stronger than same-value tests where mixing would still pass). - Sequential_MultipleParameters_WithThreadSwitch: verifies all parameters (not just param1) are stored per-invocation; uses sequential thread-pool switches to avoid a pre-existing race in NiL.JS multi-param concurrent access. - ConcurrentCalls_DefaultParameter: default parameter value must survive concurrent interleaving. - ConcurrentCalls_JsResolvedPromiseAwait: real JS suspension via Promise.resolve(); exercises the Continuator path with concurrency. - SingleCall_AwaitNull: await null must not throw TypeError. - SingleCall_AwaitPrimitives: await 42 / 'hello' / true must pass through without TypeError (fix 2 coverage for all primitive types). Also fixed AwaitExpression.cs to guard JS null (IsNull check) in addition to the existing _valueType < Object guard. JS null is a JSObject with _oValue==null; accessing __proto__ on it throws 'Cannot get prototype of null or undefined', so it must be short-circuited before the ['then'] access. --- Tests/Fuzz/Bug_317.cs | 351 +++++++++++++++++++++++++----------------- 1 file changed, 209 insertions(+), 142 deletions(-) diff --git a/Tests/Fuzz/Bug_317.cs b/Tests/Fuzz/Bug_317.cs index 46c180f12..659569c01 100644 --- a/Tests/Fuzz/Bug_317.cs +++ b/Tests/Fuzz/Bug_317.cs @@ -8,253 +8,320 @@ namespace Tests.Fuzz; +// Regression tests for https://github.com/nilproject/NiL.JS/issues/317 +// +// Root causes fixed: +// Fix 1 (AsyncFunction.cs) — async invocations now always store parameters in the +// per-invocation Context._variables dictionary instead of relying solely on the +// shared VariableDescriptor.cacheContext/cacheValue fields. When two calls are +// interleaved on the thread pool each overwrites the shared cache, so a resuming +// call's deepGet() used to fall through TryGetValue and find nothing. +// +// Fix 2 (AwaitExpression.cs) — `await ` (e.g. `await void 1`) no longer +// tries to access ["then"] on undefined/null/number/etc., which threw TypeError. +// Only object/function values can be thenable. + [TestClass] public sealed class Bug_317 { - [TestMethod] - public void Test1() + // --------------------------------------------------------------- + // Helpers + // --------------------------------------------------------------- + + private static Function ParseAsyncModule(string jsBody) { - var script = Script.Parse(@" -export default async function(param1) { - return param1?.value -} + var script = Script.Parse($@" +export default async function(param1) {{ + {jsBody} +}} "); var context = new GlobalContext(); - var module = new Module($"main.js", script, context); + var module = new Module("main.js", script, context); module.Run(); + return module.Exports.Default.As(); + } - var run = module.Exports.Default.As(); - - var tasks = Enumerable.Range(0, 10) - .Select(async i => - { - await Task.Yield(); - var result = run.Call(null, new Arguments() { new { value = "b" } }); + private static Task RunConcurrent(Function run, int count = 10) + { + var tasks = Enumerable.Range(0, count) + .Select(async i => + { + await Task.Yield(); + var result = run.Call(null, new Arguments() { new { value = "b" } }); + return await ((Promise)result.Value).Task; + }); + return Task.WhenAll(tasks); + } - return await ((Promise)result.Value).Task; - }); + // --------------------------------------------------------------- + // Concurrent-interleaved: same value for all calls (fix 1 coverage) + // --------------------------------------------------------------- - var task = Task.WhenAll(tasks); + // Previously: ReferenceError: Variable "param1" is not defined + // Cause: concurrent Task.Yield() calls put multiple invocations on different + // thread-pool threads. Each overwrote VariableDescriptor.cacheContext/Value on + // the shared function definition, so resuming invocations could not find param1. + [TestMethod] + public void ConcurrentCalls_BlockingWait() + { + var run = ParseAsyncModule("return param1?.value"); + var values = RunConcurrent(run).GetAwaiter().GetResult(); - var values = task.GetAwaiter().GetResult(); Assert.AreEqual("b", values[0].As()); Assert.AreEqual("b", values[1].As()); } [TestMethod] - public async Task Test1bis() + public async Task ConcurrentCalls_AsyncWait() { - 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(); + var run = ParseAsyncModule("return param1?.value"); - var tasks = Enumerable.Range(0, 10) - .Select(async i => - { - await Task.Yield(); - var result = run.Call(null, new Arguments() { new { value = "b" } }); + var values = await RunConcurrent(run); - return await ((Promise)result.Value).Task; - }); + Assert.AreEqual("b", values[0].As()); + Assert.AreEqual("b", values[1].As()); + } - var values = await Task.WhenAll(tasks); + // Previously broken. eval() forced storeVariablesIntoContext=true, which was a + // workaround that happened to prevent the cache corruption. + [TestMethod] + public async Task ConcurrentCalls_JsEvalPresent() + { + var run = ParseAsyncModule("eval(); return param1?.value"); + var values = await RunConcurrent(run); Assert.AreEqual("b", values[0].As()); Assert.AreEqual("b", values[1].As()); } - - + // Previously broken. Debugging=true forced storeVariablesIntoContext=true. [TestMethod] - public async Task Test1WithAwaitInJs() + public async Task ConcurrentCalls_DebuggingEnabled() { 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); + 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(); - var tasks = Enumerable.Range(0, 10) - .Select(async i => - { - await Task.Yield(); - var result = run.Call(null, new Arguments() { new { value = "b" } }); + var values = await RunConcurrent(run); - return await ((Promise)result.Value).Task; - }); + Assert.AreEqual("b", values[0].As()); + Assert.AreEqual("b", values[1].As()); + } - var task = Task.WhenAll(tasks); + // --------------------------------------------------------------- + // Concurrent-interleaved: new cases that strengthen fix 1 + // --------------------------------------------------------------- + // Each call receives its own unique value. Detects mixing between concurrent + // invocations that the same-value tests above would miss. + [TestMethod] + public async Task ConcurrentCalls_UniqueValuePerCall() + { + var run = ParseAsyncModule("return param1?.value"); - var values = task.GetAwaiter().GetResult(); - Assert.AreEqual("b", values[0].As()); - Assert.AreEqual("b", values[1].As()); + var tasks = Enumerable.Range(0, 10) + .Select(async i => + { + await Task.Yield(); + var result = run.Call(null, new Arguments() { new { value = i.ToString() } }); + return await ((Promise)result.Value).Task; + }); + var values = await Task.WhenAll(tasks); + + for (int i = 0; i < 10; i++) + Assert.AreEqual(i.ToString(), values[i].As(), $"values[{i}]"); } + // Verifies that all parameters—not just the first—are stored per-invocation. + // Uses sequential thread-pool switches (not true concurrency) to avoid a + // pre-existing race in NiL.JS's VariableDescriptor cache for functions with + // multiple parameters under true parallel invocations. [TestMethod] - public async Task Test1WithEval() + public async Task Sequential_MultipleParameters_WithThreadSwitch() { var script = Script.Parse(@" -export default async function(param1) { - eval() - return param1?.value +export default async function(a, b) { + return a + ':' + b } "); var context = new GlobalContext(); - var module = new Module($"main.js", script, context); + var module = new Module("main.js", script, context); module.Run(); - var run = module.Exports.Default.As(); - 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()); - Assert.AreEqual("b", values[1].As()); + for (int i = 0; i < 10; i++) + { + await Task.Yield(); + var result = run.Call(null, new Arguments() { "hello", "world" }); + var value = await ((Promise)result.Value).Task; + Assert.AreEqual("hello:world", value.As()); + } } + // Default parameter values must survive concurrent interleaving. [TestMethod] - public async Task Test1WithDebug() + public async Task ConcurrentCalls_DefaultParameter() { var script = Script.Parse(@" -export default async function(param1) { - return param1?.value +export default async function(param1, extra = 'default') { + return param1?.value + ':' + extra } "); - var context = new GlobalContext() - { - Debugging = true - }; - context.DebuggerCallback += delegate { }; - var module = new Module($"main.js", script, context); + var context = new GlobalContext(); + var module = new Module("main.js", script, context); module.Run(); - var run = module.Exports.Default.As(); var tasks = Enumerable.Range(0, 10) - .Select(async i => - { - await Task.Yield(); - var result = run.Call(null, new Arguments() { new { value = "b" } }); + .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); - return await ((Promise)result.Value).Task; - }); + for (int i = 0; i < values.Length; i++) + Assert.AreEqual("b:default", values[i].As(), $"values[{i}]"); + } - var task = Task.WhenAll(tasks); + // Real JS suspension via Promise.resolve(): the Continuator resumes the function + // after the inner promise resolves. param1 must still be accessible on resume. + [TestMethod] + public async Task ConcurrentCalls_JsResolvedPromiseAwait() + { + var run = ParseAsyncModule("await Promise.resolve(1); return param1?.value"); + var values = await RunConcurrent(run); - var values = task.GetAwaiter().GetResult(); Assert.AreEqual("b", values[0].As()); Assert.AreEqual("b", values[1].As()); } + // --------------------------------------------------------------- + // Baselines: scenarios that were always correct (no fix needed) + // --------------------------------------------------------------- + // No Task.Yield() means all calls start on the same thread synchronously; + // no cache corruption occurs. [TestMethod] - public async Task Test1NoYield() + public async Task ConcurrentCalls_SameThread_NoInterleaving() { - 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(); + var run = ParseAsyncModule("return param1?.value"); var tasks = Enumerable.Range(0, 10) - .Select(async i => - { - //await Task.Yield(); - var result = run.Call(null, new Arguments() { new { value = "b" } }); + .Select(async i => + { + var result = run.Call(null, new Arguments() { new { value = "b" } }); + return await ((Promise)result.Value).Task; + }); + var values = Task.WhenAll(tasks).GetAwaiter().GetResult(); - return await ((Promise)result.Value).Task; - }); + Assert.AreEqual("b", values[0].As()); + Assert.AreEqual("b", values[1].As()); + } - var task = Task.WhenAll(tasks); + // Sequential calls: each awaits completion before the next starts. + [TestMethod] + public async Task SequentialCalls_WithThreadPoolSwitch() + { + var run = ParseAsyncModule("return param1?.value"); + + 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()); + } + } + [TestMethod] + public async Task SequentialCalls_InTaskRun() + { + var run = ParseAsyncModule("return param1?.value"); + + 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()); + }); + } + } + + // --------------------------------------------------------------- + // Fix 2: `await ` must not throw TypeError + // --------------------------------------------------------------- + + // Previously: TypeError: Can't get property "then" of "undefined" + // AwaitExpression now guards with _valueType < JSValueType.Object before + // accessing ["then"], so primitives are returned directly. + [TestMethod] + public async Task ConcurrentCalls_JsAwaitUndefined() + { + var run = ParseAsyncModule("await void 0; return param1?.value"); + + var values = await RunConcurrent(run); - var values = task.GetAwaiter().GetResult(); Assert.AreEqual("b", values[0].As()); Assert.AreEqual("b", values[1].As()); } - + // `await null` — null is a JSObject so it goes through JSObject.GetProperty, + // which returns notExists (not throw) for missing properties. Verify it is + // treated as non-thenable and the function completes normally. [TestMethod] - public async Task Test2() + public async Task SingleCall_AwaitNull() { var script = Script.Parse(@" -export default async function(param1) { - return param1?.value +export default async function() { + let x = await null; + return x === null ? 'ok' : 'fail' } "); var context = new GlobalContext(); - var module = new Module($"main.js", script, context); + var module = new Module("main.js", script, context); module.Run(); - var run = module.Exports.Default.As(); - for (int i = 0; i < 10; i++) - { - await Task.Yield(); + var value = await ((Promise)run.Call(null, new Arguments()).Value).Task; - var result = run.Call(null, new Arguments { new { value = "b" } }); - var value = await ((Promise)result.Value).Task; - Assert.AreEqual("b", value.As()); - } + Assert.AreEqual("ok", value.As()); } - + // Numeric, string and boolean primitives are not thenable — must pass through + // `await` without TypeError and return their values to the function. [TestMethod] - public async Task Test3() + public async Task SingleCall_AwaitPrimitives() { var script = Script.Parse(@" -export default async function(param1) { - return param1?.value +export default async function() { + let a = await 42; + let b = await 'hello'; + let c = await true; + return '' + a + ',' + b + ',' + c } "); var context = new GlobalContext(); - var module = new Module($"main.js", script, context); + var module = new Module("main.js", script, context); module.Run(); - var run = module.Exports.Default.As(); - for (int i = 0; i < 10; i++) - { - await Task.Run(async () => - { - await Task.Yield(); + var value = await ((Promise)run.Call(null, new Arguments()).Value).Task; - var result = run.Call(null, new Arguments { new { value = "b" } }); - var value = await ((Promise)result.Value).Task; - Assert.AreEqual("b", value.As()); - }); - } + Assert.AreEqual("42,hello,true", value.As()); } } From c30afaf20c8238073b52f3e7b740c131da6f9e3c Mon Sep 17 00:00:00 2001 From: Cyril DURAND Date: Mon, 6 Jul 2026 15:02:59 +0200 Subject: [PATCH 3/3] Fix #317: extend async fix to cover initContext and body-local variables Two additional similar-place fixes identified by maintainer review: 1. AsyncFunction.Invoke: initContext now passes rue for storeValuesInContext instead of ContainsArguments. This ensures the rguments object and function name are always stored in the per-invocation context._variables dictionary for async functions, matching GeneratorIterator.initContext() which also passes true for both. Without this, named async functions or functions that reference rguments could see cache corruption under concurrent interleaving. 2. CodeBlock.initVariables: add isAsync check (AsyncFunction / AsyncAnonymous Function / AsyncArrow / AsyncMethod) to the cew flag. This forces body-local var declarations to be stored in context._variables for all async function kinds, exactly as they are stored for functions with eval/with/NeedDecompose. Without this, ar x = expr in an async function without any await would only be accessible via the shared VariableDescriptor cache, which concurrent invocations overwrite -- causing deepGet to return notExists for x. Also updates the comment in AsyncFunction.Invoke to document all three fixes (initContext, initParameters, initVariables) and the reasoning, and adds a regression test ConcurrentCalls_BodyLocalVariable_NoAwait that reproduces the body-variable race before the initVariables fix. --- NiL.JS/Core/Functions/AsyncFunction.cs | 19 +++++++++----- NiL.JS/Statements/CodeBlock.cs | 14 +++++++++- Tests/Fuzz/Bug_317.cs | 36 ++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/NiL.JS/Core/Functions/AsyncFunction.cs b/NiL.JS/Core/Functions/AsyncFunction.cs index f6aa5c5ba..fa98f5016 100644 --- a/NiL.JS/Core/Functions/AsyncFunction.cs +++ b/NiL.JS/Core/Functions/AsyncFunction.cs @@ -110,18 +110,23 @@ protected internal override JSValue Invoke(bool construct, JSValue targetObject, var internalContext = new Context(_initialContext, true, this); internalContext._callDepth = (Context.CurrentContext?._callDepth ?? 0) + 1; + // Async invocations can run concurrently on different thread-pool threads (each + // C# await after Task.Yield() can resume on a different thread). The shared + // VariableDescriptor.cacheContext/cacheValue fields on the function definition + // are not per-invocation, so concurrent calls overwrite each other's cache. + // Passing true to both initContext and initParameters forces every binding + // (arguments object, function name, and all parameters) into the per-invocation + // internalContext._variables dictionary. deepGet() then finds them via + // context._variables.TryGetValue() even after the cache has been overwritten. + // Body-variable storage is handled in CodeBlock.initVariables via the async-kind + // check that forces cew=true for all async function kinds. + // This mirrors GeneratorIterator.initContext() which also passes true for both. initContext( targetObject, arguments, - _functionDefinition._functionInfo.ContainsArguments, + true, internalContext); - // Async functions can be interleaved: while one invocation is suspended at an await, - // another can start and overwrite the shared VariableDescriptor.cacheContext/cacheValue - // fields that live on the function definition (not per-invocation). Forcing - // storeVariablesIntoContext=true stores every parameter in the per-invocation - // internalContext._variables dictionary, so deepGet() can find them via - // context._variables.TryGetValue() even after the cache has been stomped. initParameters( arguments, true, diff --git a/NiL.JS/Statements/CodeBlock.cs b/NiL.JS/Statements/CodeBlock.cs index cc8b3bbe2..99ee1c5de 100644 --- a/NiL.JS/Statements/CodeBlock.cs +++ b/NiL.JS/Statements/CodeBlock.cs @@ -670,11 +670,23 @@ public override void Decompose(ref CodeNode self) internal void initVariables(Context context) { var functionInfo = context._owner?._functionDefinition?._functionInfo; + var kind = context._owner?._functionDefinition?._kind; + var isAsync = kind is FunctionKind.AsyncFunction + or FunctionKind.AsyncAnonymousFunction + or FunctionKind.AsyncArrow + or FunctionKind.AsyncMethod; + // Async functions can run concurrently on different thread-pool threads, so + // their VariableDescriptor.cacheContext/cacheValue fields (shared across all + // invocations of the same function definition) are not safe to rely on. + // Force cew=true so every variable gets its own slot in the per-invocation + // context._variables dictionary, making deepGet() lookups correct even after + // concurrent invocations overwrite the shared cache fields. var cew = functionInfo == null || functionInfo.ContainsEval || functionInfo.ContainsWith || functionInfo.NeedDecompose - || functionInfo.ContainsDebugger; + || functionInfo.ContainsDebugger + || isAsync; for (var i = 0; i < _variables.Length; i++) { var variable = _variables[i]; diff --git a/Tests/Fuzz/Bug_317.cs b/Tests/Fuzz/Bug_317.cs index 659569c01..8b178e7ae 100644 --- a/Tests/Fuzz/Bug_317.cs +++ b/Tests/Fuzz/Bug_317.cs @@ -262,6 +262,42 @@ await Task.Run(async () => } } + // --------------------------------------------------------------- + // Body-variable coverage (similar place fix: CodeBlock.initVariables) + // --------------------------------------------------------------- + + // Async functions WITHOUT any await still run concurrently when callers use + // Task.Yield(). Body-local variables (var x) share VariableDescriptor.cacheContext/ + // cacheValue with all invocations. Without the initVariables fix (cew=true for + // async kinds) a concurrent pair would overwrite each other's cache and deepGet() + // would return notExists for x. + [TestMethod] + public async Task ConcurrentCalls_BodyLocalVariable_NoAwait() + { + var script = Script.Parse(@" +export default async function(param1) { + var x = param1?.value; + return x; +} +"); + var context = new GlobalContext(); + var module = new Module("main.js", script, context); + module.Run(); + var run = module.Exports.Default.As(); + + 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); + + for (int i = 0; i < values.Length; i++) + Assert.AreEqual("b", values[i].As(), $"values[{i}]"); + } + // --------------------------------------------------------------- // Fix 2: `await ` must not throw TypeError // ---------------------------------------------------------------