Skip to content

Commit 016204d

Browse files
avishmsftCopilot
andcommitted
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: - RC1/RC4: Use CreateService instead of GetService for ITestDataPublisher, ILegacyTestRunDataPublisher, ITestResultsServer, IFeatureFlagService, and ITestRunPublisher so each async publish gets its own service instances. - 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>
1 parent 21eed20 commit 016204d

2 files changed

Lines changed: 65 additions & 33 deletions

File tree

src/Agent.Worker/TestResults/ResultsCommandExtension.cs

Lines changed: 63 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,27 @@ public void Execute(IExecutionContext context, Command command)
7979
string teamProject = context.Variables.System_TeamProject;
8080

8181
TestRunContext runContext = CreateTestRunContext();
82+
83+
var executionContext = _executionContext;
84+
var testResultFiles = _testResultFiles;
85+
var testRunner = _testRunner;
86+
var testCaseResults = _testCaseResults;
87+
var testPlanId = _testPlanId;
88+
var runTitle = _runTitle;
89+
var publishTestResultsLibFeatureState = _publishTestResultsLibFeatureState;
90+
var triggerCoverageMergeJobFeatureState = _triggerCoverageMergeJobFeatureState;
91+
var failTaskOnFailedTests = _failTaskOnFailedTests;
92+
var publishOptions = GetPublishOptions();
93+
var telemetryProperties = _telemetryProperties;
94+
8295
var commandContext = context.GetHostContext().CreateService<IAsyncCommandContext>();
8396
commandContext.InitializeCommandContext(context, StringUtil.Loc("PublishTestResults"));
84-
commandContext.Task = PublishTestRunDataAsync(teamProject, runContext);
85-
_executionContext.AsyncCommands.Add(commandContext);
97+
commandContext.Task = PublishTestRunDataAsync(
98+
teamProject, runContext, executionContext, testResultFiles, testRunner,
99+
testCaseResults, testPlanId, runTitle, publishTestResultsLibFeatureState,
100+
triggerCoverageMergeJobFeatureState, failTaskOnFailedTests,
101+
publishOptions, telemetryProperties);
102+
executionContext.AsyncCommands.Add(commandContext);
86103
}
87104

88105
private void LoadPublishTestResultsInputs(IExecutionContext context, Dictionary<string, string> eventProperties, string data)
@@ -303,47 +320,62 @@ private PublishOptions GetPublishOptions()
303320
return publishOptions;
304321
}
305322

306-
private async Task PublishTestRunDataAsync(string teamProject, TestRunContext testRunContext)
323+
private async Task PublishTestRunDataAsync(
324+
string teamProject,
325+
TestRunContext testRunContext,
326+
IExecutionContext executionContext,
327+
List<string> testResultFiles,
328+
string testRunner,
329+
TestCaseResult[] testCaseResults,
330+
string testPlanId,
331+
string runTitle,
332+
bool publishTestResultsLibFeatureState,
333+
bool triggerCoverageMergeJobFeatureState,
334+
bool failTaskOnFailedTests,
335+
PublishOptions publishOptions,
336+
Dictionary<string, object> telemetryProperties)
307337
{
308338
bool isTestRunOutcomeFailed = false;
309339

310-
_telemetryProperties.Add("UsePublishTestResultsLib", _publishTestResultsLibFeatureState);
311-
using (var connection = WorkerUtilities.GetVssConnection(_executionContext))
340+
telemetryProperties.Add("UsePublishTestResultsLib", publishTestResultsLibFeatureState);
341+
using (var connection = WorkerUtilities.GetVssConnection(executionContext))
312342
{
313-
314-
//This check is to determine to use "Microsoft.TeamFoundation.PublishTestResults" Library or the agent code to parse and publish the test results.
315-
if (_publishTestResultsLibFeatureState)
343+
// Use CreateService instead of GetService to get per-invocation instances.
344+
// GetService returns a cached singleton, so a second Execute() call would
345+
// reinitialize the same instance while the first async task is still in-flight,
346+
// causing ObjectDisposedException when the second using block disposes the connection.
347+
if (publishTestResultsLibFeatureState)
316348
{
317-
var publisher = _executionContext.GetHostContext().GetService<ITestDataPublisher>();
318-
publisher.InitializePublisher(_executionContext, teamProject, connection, _testRunner);
349+
var publisher = executionContext.GetHostContext().CreateService<ITestDataPublisher>();
350+
publisher.InitializePublisher(executionContext, teamProject, connection, testRunner);
319351

320-
if (!_testCaseResults.IsNullOrEmpty() && !_testPlanId.IsNullOrEmpty())
352+
if (!testCaseResults.IsNullOrEmpty() && !testPlanId.IsNullOrEmpty())
321353
{
322-
isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, _testResultFiles, _testCaseResults, GetPublishOptions(), _executionContext.CancellationToken);
354+
isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, testResultFiles, testCaseResults, publishOptions, executionContext.CancellationToken);
323355
}
324356
else
325357
{
326-
isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, _testResultFiles, GetPublishOptions(), _executionContext.CancellationToken);
358+
isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, testResultFiles, publishOptions, executionContext.CancellationToken);
327359
}
328360
}
329361
else
330362
{
331-
var publisher = _executionContext.GetHostContext().GetService<ILegacyTestRunDataPublisher>();
332-
publisher.InitializePublisher(_executionContext, teamProject, connection, _testRunner, _publishRunLevelAttachments);
363+
var publisher = executionContext.GetHostContext().CreateService<ILegacyTestRunDataPublisher>();
364+
publisher.InitializePublisher(executionContext, teamProject, connection, testRunner, publishOptions.IsAddTestRunAttachments);
333365

334-
isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, _testResultFiles, _runTitle, _executionContext.Variables.Build_BuildId, _mergeResults);
366+
isTestRunOutcomeFailed = await publisher.PublishAsync(testRunContext, testResultFiles, runTitle, executionContext.Variables.Build_BuildId, publishOptions.IsMergeTestResultsToSingleRun);
335367
}
336368

337-
if (isTestRunOutcomeFailed && _failTaskOnFailedTests)
369+
if (isTestRunOutcomeFailed && failTaskOnFailedTests)
338370
{
339-
_executionContext.Result = TaskResult.Failed;
340-
_executionContext.Error(StringUtil.Loc("FailedTestsInResults"));
371+
executionContext.Result = TaskResult.Failed;
372+
executionContext.Error(StringUtil.Loc("FailedTestsInResults"));
341373
}
342374

343-
await PublishEventsAsync(connection);
344-
if (_triggerCoverageMergeJobFeatureState)
375+
await PublishEventsAsync(connection, executionContext, telemetryProperties);
376+
if (triggerCoverageMergeJobFeatureState)
345377
{
346-
TriggerCoverageMergeJob(_testResultFiles, _executionContext);
378+
TriggerCoverageMergeJob(testResultFiles, executionContext);
347379
}
348380
}
349381
}
@@ -354,7 +386,7 @@ private void TriggerCoverageMergeJob(List<string> resultFilesInput, IExecutionCo
354386
try
355387
{
356388
ITestResultsServer _testResultsServer = context.GetHostContext().GetService<ITestResultsServer>();
357-
using (var connection = WorkerUtilities.GetVssConnection(_executionContext))
389+
using (var connection = WorkerUtilities.GetVssConnection(context))
358390
{
359391
foreach (var resultFile in resultFilesInput)
360392
{
@@ -374,14 +406,14 @@ private void TriggerCoverageMergeJob(List<string> resultFilesInput, IExecutionCo
374406
Path.GetExtension(file).Equals(".coverage", StringComparison.OrdinalIgnoreCase)
375407
)
376408
{
377-
_testResultsServer.InitializeServer(connection, _executionContext);
409+
_testResultsServer.InitializeServer(connection, context);
378410
try
379411
{
380-
var codeCoverageResults = _testResultsServer.UpdateCodeCoverageSummaryAsync(connection, _executionContext.Variables.System_TeamProjectId.ToString(), _executionContext.Variables.Build_BuildId.GetValueOrDefault());
412+
var codeCoverageResults = _testResultsServer.UpdateCodeCoverageSummaryAsync(connection, context.Variables.System_TeamProjectId.ToString(), context.Variables.Build_BuildId.GetValueOrDefault());
381413
}
382414
catch (Exception e)
383415
{
384-
_executionContext.Section($"Could not queue code coverage merge:{e}");
416+
context.Section($"Could not queue code coverage merge:{e}");
385417
}
386418
}
387419
}
@@ -391,28 +423,28 @@ private void TriggerCoverageMergeJob(List<string> resultFilesInput, IExecutionCo
391423
}
392424
catch (Exception e)
393425
{
394-
_executionContext.Debug($"Exception in Method:{e.Message}");
426+
context.Debug($"Exception in Method:{e.Message}");
395427
}
396428
}
397429

398-
private async Task PublishEventsAsync(VssConnection connection)
430+
private async Task PublishEventsAsync(VssConnection connection, IExecutionContext executionContext, Dictionary<string, object> telemetryProperties)
399431
{
400432
try
401433
{
402434
CustomerIntelligenceEvent ciEvent = new CustomerIntelligenceEvent()
403435
{
404436
Area = _telemetryArea,
405437
Feature = _telemetryFeature,
406-
Properties = _telemetryProperties
438+
Properties = telemetryProperties
407439
};
408440

409-
var ciService = _executionContext.GetHostContext().GetService<ICustomerIntelligenceServer>();
441+
var ciService = executionContext.GetHostContext().GetService<ICustomerIntelligenceServer>();
410442
ciService.Initialize(connection);
411443
await ciService.PublishEventsAsync(new CustomerIntelligenceEvent[] { ciEvent });
412444
}
413445
catch (Exception ex)
414446
{
415-
_executionContext.Debug(StringUtil.Loc("TelemetryCommandFailed", ex.Message));
447+
executionContext.Debug(StringUtil.Loc("TelemetryCommandFailed", ex.Message));
416448
}
417449
}
418450

src/Agent.Worker/TestResults/TestDataPublisher.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ public void InitializePublisher(IExecutionContext context, string projectName, V
5454
_testRunner = testRunner;
5555
_testRunPublisher = new TestRunPublisher(connection, new CommandTraceListener(context));
5656
_testLogStore = new TestLogStore(connection, new CommandTraceListener(context));
57-
_testResultsServer = HostContext.GetService<ITestResultsServer>();
57+
_testResultsServer = HostContext.CreateService<ITestResultsServer>();
5858
_testResultsServer.InitializeServer(connection, _executionContext);
5959
var extensionManager = HostContext.GetService<IExtensionManager>();
60-
_featureFlagService = HostContext.GetService<IFeatureFlagService>();
60+
_featureFlagService = HostContext.CreateService<IFeatureFlagService>();
6161
_featureFlagService.InitializeFeatureService(_executionContext, connection);
6262
_calculateTestRunSummary = _featureFlagService.GetFeatureFlagState(TestResultsConstants.CalculateTestRunSummaryFeatureFlag, TestResultsConstants.TFSServiceInstanceGuid);
6363
_isFlakyCheckEnabled = _featureFlagService.GetFeatureFlagState(TestResultsConstants.EnableFlakyCheckInAgentFeatureFlag, TestResultsConstants.TCMServiceInstanceGuid); ;

0 commit comments

Comments
 (0)