Task.Run does not guarantee using new thread

Author: vmoroz

.ado/publish.yml

@@ -259,7 +259,6 @@ extends:
                 - script: >
                     dotnet test -f ${{ MatrixEntry.DotNetVersion }}
                     --configuration Release
-                    --property:ParallelizeTestCollections=false
                     --logger trx
                     --results-directory "$(Build.StagingDirectory)/test/${{ MatrixEntry.DotNetVersion }}-Release"
                   displayName: Run tests

.github/workflows/build.yml

@@ -77,7 +77,6 @@ jobs:
       run: >
         dotnet test -f ${{ matrix.dotnet-version }}
         --configuration ${{ matrix.configuration }}
-        --property:ParallelizeTestCollections=false
         --logger trx
         --results-directory "out/test/${{matrix.dotnet-version}}-node${{matrix.node-version}}-${{matrix.configuration}}"
       continue-on-error: true

Directory.Packages.props

@@ -17,7 +17,6 @@
     <PackageVersion Include="System.Reflection.MetadataLoadContext" Version="6.0.0" />
     <PackageVersion Include="xunit" Version="2.4.2" />
     <PackageVersion Include="xunit.runner.visualstudio" Version="2.4.5" />
-    <PackageVersion Include="Xunit.SkippableFact" Version="1.4.13" />
     <PackageVersion Include="XmlDocMarkdown.Core" Version="2.9.0" />
   </ItemGroup>
 </Project>

test/GCTests.cs

@@ -12,12 +12,9 @@ public class GCTests
 {
     private static string LibnodePath { get; } = GetLibnodePath();
 
-    [SkippableFact]
+    [Fact]
     public void GCHandles()
     {
-        Skip.If(
-            NodejsEmbeddingTests.NodejsPlatform == null,
-            "Node shared library not found at " + LibnodePath);
         using NodeEmbeddingThreadRuntime nodejs = NodejsEmbeddingTests.CreateNodejsEnvironment();
 
         nodejs.Run(() =>
@@ -63,12 +60,9 @@ public void GCHandles()
         });
     }
 
-    [SkippableFact]
+    [Fact]
     public void GCObjects()
     {
-        Skip.If(
-            NodejsEmbeddingTests.NodejsPlatform == null,
-            "Node shared library not found at " + LibnodePath);
         using NodeEmbeddingThreadRuntime nodejs = NodejsEmbeddingTests.CreateNodejsEnvironment();
 
         nodejs.Run(() =>

test/JSReferenceTests.cs

@@ -2,7 +2,6 @@
 // Licensed under the MIT License.
 
 using System;
-using System.Threading.Tasks;
 using Xunit;
 using static Microsoft.JavaScript.NodeApi.Runtime.JSRuntime;
 
@@ -52,7 +51,7 @@ public void GetReferenceFromDifferentThread()
         JSReference reference = new(value);
 
         // Run in a new thread which will not have any current scope.
-        Task.Run(() =>
+        TestUtils.RunInThread(() =>
         {
             Assert.Throws<JSInvalidThreadAccessException>(() => reference.GetValue());
         }).Wait();
@@ -67,7 +66,7 @@ public void GetReferenceFromDifferentRootScope()
         JSReference reference = new(value);
 
         // Run in a new thread and establish another root scope there.
-        Task.Run(() =>
+        TestUtils.RunInThread(() =>
         {
             using JSValueScope rootScope2 = TestScope(JSValueScopeType.Root);
             Assert.Throws<JSInvalidThreadAccessException>(() => reference.GetValue());

test/JSValueScopeTests.cs

@@ -2,7 +2,6 @@
 // Licensed under the MIT License.
 
 using System;
-using System.Threading.Tasks;
 using Xunit;
 using static Microsoft.JavaScript.NodeApi.Runtime.JSRuntime;
 
@@ -325,7 +324,7 @@ public void CreateValueFromDifferentThread()
         using JSValueScope rootScope = TestScope(JSValueScopeType.Root);
 
         // Run in a new thread which will not have any current scope.
-        Task.Run(() =>
+        TestUtils.RunInThread(() =>
         {
             Assert.Throws<JSInvalidThreadAccessException>(() => JSValueScope.Current);
             JSInvalidThreadAccessException ex = Assert.Throws<JSInvalidThreadAccessException>(
@@ -342,7 +341,7 @@ public void AccessValueFromDifferentThread()
         JSValue objectValue = JSValue.CreateObject();
 
         // Run in a new thread which will not have any current scope.
-        Task.Run(() =>
+        TestUtils.RunInThread(() =>
         {
             Assert.Throws<JSInvalidThreadAccessException>(() => JSValueScope.Current);
             JSInvalidThreadAccessException ex = Assert.Throws<JSInvalidThreadAccessException>(
@@ -359,7 +358,7 @@ public void AccessValueFromDifferentRootScope()
         JSValue objectValue = JSValue.CreateObject();
 
         // Run in a new thread and establish another root scope there.
-        Task.Run(() =>
+        TestUtils.RunInThread(() =>
         {
             using JSValueScope rootScope2 = TestScope(JSValueScopeType.Root);
             Assert.Equal(JSValueScopeType.Root, JSValueScope.Current.ScopeType);

test/NodeApi.Test.csproj

@@ -24,7 +24,6 @@
     <PackageReference Include="Microsoft.NET.Test.Sdk" />
     <PackageReference Include="xunit" />
     <PackageReference Include="xunit.runner.visualstudio" />
-    <PackageReference Include="Xunit.SkippableFact" />
   </ItemGroup>
 
   <ItemGroup>

test/TestCases/napi-dotnet/ThreadSafety.cs

@@ -1,8 +1,9 @@
-// Copyright (c) Microsoft Corporation.
+// Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
 using System;
 using System.Collections.Generic;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace Microsoft.JavaScript.NodeApi.TestCases;
@@ -36,7 +37,7 @@ private static void ValidateNotOnJSThread()
 
     public static async Task CallDelegateFromOtherThread(Action action)
     {
-        await Task.Run(() =>
+        await RunInThread(() =>
         {
             ValidateNotOnJSThread();
 
@@ -48,7 +49,7 @@ public static async Task<string> CallInterfaceMethodFromOtherThread(
         ISmpleInterface interfaceObj,
         string value)
     {
-        return await Task.Run(() =>
+        return await RunInThread(() =>
         {
             ValidateNotOnJSThread();
 
@@ -59,7 +60,7 @@ public static async Task<string> CallInterfaceMethodFromOtherThread(
     public static async Task<int> EnumerateCollectionFromOtherThread(
         IReadOnlyCollection<int> collection)
     {
-        return await Task.Run(() =>
+        return await RunInThread(() =>
         {
             ValidateNotOnJSThread();
 
@@ -76,7 +77,7 @@ public static async Task<int> EnumerateCollectionFromOtherThread(
     public static async Task<int> EnumerateDictionaryFromOtherThread(
         IReadOnlyDictionary<string, string> dictionary)
     {
-        return await Task.Run(() =>
+        return await RunInThread(() =>
         {
             ValidateNotOnJSThread();
 
@@ -93,11 +94,52 @@ public static async Task<int> EnumerateDictionaryFromOtherThread(
     public static async Task<bool> ModifyDictionaryFromOtherThread(
         IDictionary<string, string> dictionary, string keyToRemove)
     {
-        return await Task.Run(() =>
+        return await RunInThread(() =>
         {
             ValidateNotOnJSThread();
 
             return dictionary.Remove(keyToRemove);
         });
     }
+
+    private static Task RunInThread(Action action)
+    {
+        TaskCompletionSource<bool> threadCompletion = new TaskCompletionSource<bool>();
+
+        Thread thread = new Thread(() =>
+        {
+            try
+            {
+                action();
+                threadCompletion.TrySetResult(true);
+            }
+            catch (Exception e)
+            {
+                threadCompletion.TrySetException(e);
+            }
+        });
+        thread.Start();
+
+        return threadCompletion.Task;
+    }
+
+    private static Task<T> RunInThread<T>(Func<T> func)
+    {
+        TaskCompletionSource<T> threadCompletion = new TaskCompletionSource<T>();
+
+        Thread thread = new Thread(() =>
+        {
+            try
+            {
+                threadCompletion.TrySetResult(func());
+            }
+            catch (Exception e)
+            {
+                threadCompletion.TrySetException(e);
+            }
+        });
+        thread.Start();
+
+        return threadCompletion.Task;
+    }
 }

test/TestUtils.cs

@@ -7,6 +7,7 @@
 using System.Runtime.InteropServices;
 using System.Text;
 using System.Threading;
+using System.Threading.Tasks;
 
 namespace Microsoft.JavaScript.NodeApi.Test;
 
@@ -145,4 +146,25 @@ public static void CopyIfNewer(string sourceFilePath, string targetFilePath)
             File.Copy(sourceFilePath, targetFilePath, overwrite: true);
         }
     }
+
+    public static Task RunInThread(Action action)
+    {
+        TaskCompletionSource<bool> threadCompletion = new TaskCompletionSource<bool>();
+
+        Thread thread = new Thread(() =>
+        {
+            try
+            {
+                action();
+                threadCompletion.TrySetResult(true);
+            }
+            catch (Exception e)
+            {
+                threadCompletion.TrySetException(e);
+            }
+        });
+        thread.Start();
+
+        return threadCompletion.Task;
+    }
 }