Fix ObjectDisposedException race in PublishTestResultsCommand

Concurrent ##vso[results.publish] commands caused sporadic
ObjectDisposedException because async tasks continued reading shared
singleton state after the command lock released and the next Execute()
overwrote it.

Changes:
- Create VssConnection once in Execute() and share it between
  LoadFeatureFlagState and the async publish task. Previously
  LoadFeatureFlagState created its own connection in a using block that
  disposed immediately, poisoning the singleton FeatureFlagService's
  connection. Parsers (JUnitParser, etc.) later called
  GetService<IFeatureFlagService>() and hit the dead connection.
  The async task takes ownership and disposes the connection when done.
- RC1/RC4: Use CreateService instead of GetService for ITestDataPublisher,
  ITestResultsServer, and IFeatureFlagService so each async publish gets
  its own service instances (modern publisher path).
- RC2: Capture all mutable instance fields as locals in Execute() before
  the async continuation to prevent subsequent calls from corrupting
  in-flight state.
- Pass captured state as parameters to PublishTestRunDataAsync and helper
  methods instead of reading instance fields.
- TriggerCoverageMergeJob: use passed-in locals instead of instance fields
  but preserve original fire-and-forget behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: avishmsft

src/Agent.Worker/TestResults/ResultsCommandExtension.cs

@@ -72,17 +72,42 @@ public void Execute(IExecutionContext context, Command command)
 
             _telemetryProperties = new Dictionary<string, object>();
             PopulateTelemetryData();
-            LoadFeatureFlagState();
+
+            // Create the connection once and share it between LoadFeatureFlagState and the
+            // async publish task. Previously LoadFeatureFlagState created its own connection
+            // in a using block that disposed immediately, poisoning the singleton
+            // FeatureFlagService's _connection. Parsers (JUnitParser, etc.) later called
+            // GetService<IFeatureFlagService>() and hit the dead connection.
+            // The async task takes ownership of this connection and disposes it when done.
+            var connection = WorkerUtilities.GetVssConnection(context);
+            LoadFeatureFlagState(connection);
 
             LoadPublishTestResultsInputs(context, eventProperties, data);
 
             string teamProject = context.Variables.System_TeamProject;
 
             TestRunContext runContext = CreateTestRunContext();
+
+            var executionContext = _executionContext;
+            var testResultFiles = _testResultFiles;
+            var testRunner = _testRunner;
+            var testCaseResults = _testCaseResults;
+            var testPlanId = _testPlanId;
+            var runTitle = _runTitle;
+            var publishTestResultsLibFeatureState = _publishTestResultsLibFeatureState;
+            var triggerCoverageMergeJobFeatureState = _triggerCoverageMergeJobFeatureState;
+            var failTaskOnFailedTests = _failTaskOnFailedTests;
+            var publishOptions = GetPublishOptions();
+            var telemetryProperties = _telemetryProperties;
+
             var commandContext = context.GetHostContext().CreateService<IAsyncCommandContext>();
             commandContext.InitializeCommandContext(context, StringUtil.Loc("PublishTestResults"));
-            commandContext.Task = PublishTestRunDataAsync(teamProject, runContext);
-            _executionContext.AsyncCommands.Add(commandContext);
+            commandContext.Task = PublishTestRunDataAsync(
+                teamProject, runContext, executionContext, testResultFiles, testRunner,
+                testCaseResults, testPlanId, runTitle, publishTestResultsLibFeatureState,
+                triggerCoverageMergeJobFeatureState, failTaskOnFailedTests,
+                publishOptions, telemetryProperties, connection);
+            executionContext.AsyncCommands.Add(commandContext);
         }
 
         private void LoadPublishTestResultsInputs(IExecutionContext context, Dictionary<string, string> eventProperties, string data)
@@ -303,47 +328,63 @@ private PublishOptions GetPublishOptions()
             return publishOptions;
         }
 
-        private async Task PublishTestRunDataAsync(string teamProject, TestRunContext testRunContext)
+        private async Task PublishTestRunDataAsync(
+            string teamProject,
+            TestRunContext testRunContext,
+            IExecutionContext executionContext,
+            List<string> testResultFiles,
+            string testRunner,
+            TestCaseResult[] testCaseResults,
+            string testPlanId,
+            string runTitle,
+            bool publishTestResultsLibFeatureState,
+            bool triggerCoverageMergeJobFeatureState,
+            bool failTaskOnFailedTests,
+            PublishOptions publishOptions,
+            Dictionary<string, object> telemetryProperties,
+            VssConnection connection)
         {
             bool isTestRunOutcomeFailed = false;
 
-            _telemetryProperties.Add("UsePublishTestResultsLib", _publishTestResultsLibFeatureState);
-            using (var connection = WorkerUtilities.GetVssConnection(_executionContext))
+            telemetryProperties.Add("UsePublishTestResultsLib", publishTestResultsLibFeatureState);
+            using (connection)
             {
-
-                //This check is to determine to use "Microsoft.TeamFoundation.PublishTestResults" Library or the agent code to parse and publish the test results.
-                if (_publishTestResultsLibFeatureState)
+                // Use CreateService instead of GetService to get per-invocation instances.
+                // GetService returns a cached singleton, so a second Execute() call would
+                // reinitialize the same instance while the first async task is still in-flight,
+                // causing ObjectDisposedException when the second using block disposes the connection.
+                if (publishTestResultsLibFeatureState)
                 {
-                    var publisher = _executionContext.GetHostContext().GetService<ITestDataPublisher>();
-                    publisher.InitializePublisher(_executionContext, teamProject, connection, _testRunner);
+                    var publisher = executionContext.GetHostContext().CreateService<ITestDataPublisher>();
+                    publisher.InitializePublisher(executionContext, teamProject, connection, testRunner);
 
-                    if (!_testCaseResults.IsNullOrEmpty() && !_testPlanId.IsNullOrEmpty())
+                    if (!testCaseResults.IsNullOrEmpty() && !testPlanId.IsNullOrEmpty())
                     {
-                        isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, _testResultFiles, _testCaseResults, GetPublishOptions(), _executionContext.CancellationToken);
+                        isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, testResultFiles, testCaseResults, publishOptions, executionContext.CancellationToken);
                     }
                     else
                     {
-                        isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, _testResultFiles, GetPublishOptions(), _executionContext.CancellationToken);
+                        isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, testResultFiles, publishOptions, executionContext.CancellationToken);
                     }
                 }
                 else
                 {
-                    var publisher = _executionContext.GetHostContext().GetService<ILegacyTestRunDataPublisher>();
-                    publisher.InitializePublisher(_executionContext, teamProject, connection, _testRunner, _publishRunLevelAttachments);
+                    var publisher = executionContext.GetHostContext().CreateService<ILegacyTestRunDataPublisher>();
+                    publisher.InitializePublisher(executionContext, teamProject, connection, testRunner, publishOptions.IsAddTestRunAttachments);
 
-                    isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, _testResultFiles, _runTitle, _executionContext.Variables.Build_BuildId, _mergeResults);
+                    isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, testResultFiles, runTitle, executionContext.Variables.Build_BuildId, publishOptions.IsMergeTestResultsToSingleRun);
                 }
 
-                if (isTestRunOutcomeFailed && _failTaskOnFailedTests)
+                if (isTestRunOutcomeFailed && failTaskOnFailedTests)
                 {
-                    _executionContext.Result = TaskResult.Failed;
-                    _executionContext.Error(StringUtil.Loc("FailedTestsInResults"));
+                    executionContext.Result = TaskResult.Failed;
+                    executionContext.Error(StringUtil.Loc("FailedTestsInResults"));
                 }
 
-                await PublishEventsAsync(connection);
-                if (_triggerCoverageMergeJobFeatureState)
+                await PublishEventsAsync(connection, executionContext, telemetryProperties);
+                if (triggerCoverageMergeJobFeatureState)
                 {
-                    TriggerCoverageMergeJob(_testResultFiles, _executionContext);
+                    TriggerCoverageMergeJob(testResultFiles, executionContext);
                 }
             }
         }
@@ -354,7 +395,7 @@ private void TriggerCoverageMergeJob(List<string> resultFilesInput, IExecutionCo
             try
             {
                 ITestResultsServer _testResultsServer = context.GetHostContext().GetService<ITestResultsServer>();
-                using (var connection = WorkerUtilities.GetVssConnection(_executionContext))
+                using (var connection = WorkerUtilities.GetVssConnection(context))
                 {
                     foreach (var resultFile in resultFilesInput)
                     {
@@ -374,14 +415,14 @@ private void TriggerCoverageMergeJob(List<string> resultFilesInput, IExecutionCo
                                     Path.GetExtension(file).Equals(".coverage", StringComparison.OrdinalIgnoreCase)
                                     )
                                 {
-                                    _testResultsServer.InitializeServer(connection, _executionContext);
+                                    _testResultsServer.InitializeServer(connection, context);
                                     try
                                     {
-                                        var codeCoverageResults = _testResultsServer.UpdateCodeCoverageSummaryAsync(connection, _executionContext.Variables.System_TeamProjectId.ToString(), _executionContext.Variables.Build_BuildId.GetValueOrDefault());
+                                        var codeCoverageResults = _testResultsServer.UpdateCodeCoverageSummaryAsync(connection, context.Variables.System_TeamProjectId.ToString(), context.Variables.Build_BuildId.GetValueOrDefault());
                                     }
                                     catch (Exception e)
                                     {
-                                        _executionContext.Section($"Could not queue code coverage merge:{e}");
+                                        context.Section($"Could not queue code coverage merge:{e}");
                                     }
                                 }
                             }
@@ -391,28 +432,28 @@ private void TriggerCoverageMergeJob(List<string> resultFilesInput, IExecutionCo
             }
             catch (Exception e)
             {
-                _executionContext.Debug($"Exception in Method:{e.Message}");
+                context.Debug($"Exception in Method:{e.Message}");
             }
         }
 
-        private async Task PublishEventsAsync(VssConnection connection)
+        private async Task PublishEventsAsync(VssConnection connection, IExecutionContext executionContext, Dictionary<string, object> telemetryProperties)
         {
             try
             {
                 CustomerIntelligenceEvent ciEvent = new CustomerIntelligenceEvent()
                 {
                     Area = _telemetryArea,
                     Feature = _telemetryFeature,
-                    Properties = _telemetryProperties
+                    Properties = telemetryProperties
                 };
 
-                var ciService = _executionContext.GetHostContext().GetService<ICustomerIntelligenceServer>();
+                var ciService = executionContext.GetHostContext().GetService<ICustomerIntelligenceServer>();
                 ciService.Initialize(connection);
                 await ciService.PublishEventsAsync(new CustomerIntelligenceEvent[] { ciEvent });
             }
             catch (Exception ex)
             {
-                _executionContext.Debug(StringUtil.Loc("TelemetryCommandFailed", ex.Message));
+                executionContext.Debug(StringUtil.Loc("TelemetryCommandFailed", ex.Message));
             }
         }
 
@@ -432,15 +473,12 @@ private void PopulateTelemetryData()
             }
         }
 
-        private void LoadFeatureFlagState()
+        private void LoadFeatureFlagState(VssConnection connection)
         {
-            using (var connection = WorkerUtilities.GetVssConnection(_executionContext))
-            {
-                var featureFlagService = _executionContext.GetHostContext().GetService<IFeatureFlagService>();
-                featureFlagService.InitializeFeatureService(_executionContext, connection);
-                _publishTestResultsLibFeatureState = featureFlagService.GetFeatureFlagState(TestResultsConstants.UsePublishTestResultsLibFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid);
-                _triggerCoverageMergeJobFeatureState = featureFlagService.GetFeatureFlagState(CodeCoverageConstants.TriggerCoverageMergeJobFF, TestResultsConstants.TFSServiceInstanceGuid);
-            }
+            var featureFlagService = _executionContext.GetHostContext().GetService<IFeatureFlagService>();
+            featureFlagService.InitializeFeatureService(_executionContext, connection);
+            _publishTestResultsLibFeatureState = featureFlagService.GetFeatureFlagState(TestResultsConstants.UsePublishTestResultsLibFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid);
+            _triggerCoverageMergeJobFeatureState = featureFlagService.GetFeatureFlagState(CodeCoverageConstants.TriggerCoverageMergeJobFF, TestResultsConstants.TFSServiceInstanceGuid);
         }
     }
 

src/Agent.Worker/TestResults/TestDataPublisher.cs

@@ -54,10 +54,10 @@ public void InitializePublisher(IExecutionContext context, string projectName, V
             _testRunner = testRunner;
             _testRunPublisher = new TestRunPublisher(connection, new CommandTraceListener(context));
             _testLogStore = new TestLogStore(connection, new CommandTraceListener(context));
-            _testResultsServer = HostContext.GetService<ITestResultsServer>();
+            _testResultsServer = HostContext.CreateService<ITestResultsServer>();
             _testResultsServer.InitializeServer(connection, _executionContext);
             var extensionManager = HostContext.GetService<IExtensionManager>();
-            _featureFlagService = HostContext.GetService<IFeatureFlagService>();
+            _featureFlagService = HostContext.CreateService<IFeatureFlagService>();
             _featureFlagService.InitializeFeatureService(_executionContext, connection);
             _calculateTestRunSummary = _featureFlagService.GetFeatureFlagState(TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid);
             _isFlakyCheckEnabled = _featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid); ;