Concurrency in parallel runs. (#927)

Use a concurrent dictionary for managing the proxy managers and handlers for parallel runs. It is possible that the child runs end up at the same time. It is necessary for ParallelProxyOperationManager to be thread safe. Remove MemoryTraceWriter for data serializer since it is not thread safe. Enabled the parallel integration tests.
microsoft · Jul 14, 2017 · a08c38a · a08c38a
1 parent e2ac8ce
commit a08c38a
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 4 deletions.
diff --git a/src/Microsoft.TestPlatform.CommunicationUtilities/JsonDataSerializer.cs b/src/Microsoft.TestPlatform.CommunicationUtilities/JsonDataSerializer.cs
@@ -42,9 +42,11 @@ private JsonDataSerializer()
             payloadSerializer.ContractResolver = new TestPlatformContractResolver1();
             payloadSerializer2.ContractResolver = new DefaultTestPlatformContractResolver();
 
-#if DEBUG
+#if TRACE_JSON_SERIALIZATION
             // MemoryTraceWriter can help diagnose serialization issues. Enable it for
             // debug builds only.
+            // Note that MemoryTraceWriter is not thread safe, please don't use it in parallel
+            // test runs. See https://github.com/JamesNK/Newtonsoft.Json/issues/1279
             payloadSerializer.TraceWriter = new MemoryTraceWriter();
             payloadSerializer2.TraceWriter = new MemoryTraceWriter();
 #endif

diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelOperationManager.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelOperationManager.cs
@@ -10,6 +10,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client
     using System.Threading.Tasks;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine;
+    using System.Collections.Concurrent;
 
     /// <summary>
     /// Abstract class having common parallel manager implementation
@@ -132,7 +133,7 @@ public void UpdateParallelLevel(int newParallelLevel)
             {
                 // not initialized yet
                 // create rest of concurrent clients other than default one
-                this.concurrentManagerHandlerMap = new Dictionary<T, U>();
+                this.concurrentManagerHandlerMap = new ConcurrentDictionary<T, U>();
                 for (int i = 0; i < newParallelLevel; i++)
                 {
                     this.AddManager(this.CreateNewConcurrentManager(), default(U));

diff --git a/test/Microsoft.TestPlatform.AcceptanceTests/ExecutionTests.cs b/test/Microsoft.TestPlatform.AcceptanceTests/ExecutionTests.cs
@@ -30,7 +30,6 @@ public void RunMultipleTestAssemblies(string runnerFramework, string targetFrame
         // Randomly failing with error "The active test run was aborted. Reason: Destination array was not long enough.
         // Check destIndex and length, and the array's lower bounds. Test Run Failed."
         // Issue: https://github.com/Microsoft/vstest/issues/292
-        [Ignore]
         [CustomDataTestMethod]
         [NET46TargetFramework]
         [NETCORETargetFramework]

diff --git a/test/Microsoft.TestPlatform.AcceptanceTests/RunsettingsTests.cs b/test/Microsoft.TestPlatform.AcceptanceTests/RunsettingsTests.cs
@@ -195,7 +195,6 @@ public void RunSettingsAndRunSettingsParamsAsArguments(string runnerFramework, s
         // Randomly failing with error "The active test run was aborted. Reason: Destination array was not long enough.
         // Check destIndex and length, and the array's lower bounds. Test Run Failed."
         // Issue: https://github.com/Microsoft/vstest/issues/292
-        [Ignore]
         [CustomDataTestMethod]
         [NET46TargetFramework]
         [NETCORETargetFramework]