Skip to content

Commit

Permalink
Check overall build success in one place (#8953)
Browse files Browse the repository at this point in the history
Fixes #8735, possibly #5689, #2845

Context
There were race condition when ProjectFinished log message has been processed after BuildResult event. Even though ProjectFinished log message event is always send before BuildResult event from Node, because ProjectFinished is asyncronously routed by LoggingService reversing order can and do often happen.

Changes Made
Updating _overallBuildSuccess in one place when submission is considered to be done and assumed its final state.

Testing
Can't local repro.
  • Loading branch information
rokonec committed Jun 27, 2023
1 parent a0468d8 commit c8c2aaa
Showing 1 changed file with 2 additions and 6 deletions.
8 changes: 2 additions & 6 deletions src/Build/BackEnd/BuildManager/BuildManager.cs
Expand Up @@ -2466,7 +2466,6 @@ private void CheckForActiveNodesAndCleanUpSubmissions()
// shut down.
submission.CompleteLogging();

_overallBuildSuccess = _overallBuildSuccess && (submission.BuildResult.OverallResult == BuildResultCode.Success);
CheckSubmissionCompletenessAndRemove(submission);
}

Expand All @@ -2480,7 +2479,6 @@ private void CheckForActiveNodesAndCleanUpSubmissions()

submission.CompleteResults(new GraphBuildResult(submission.SubmissionId, new BuildAbortedException()));

_overallBuildSuccess &= submission.BuildResult.OverallResult == BuildResultCode.Success;
CheckSubmissionCompletenessAndRemove(submission);
}

Expand Down Expand Up @@ -2592,8 +2590,6 @@ private void ReportResultsToSubmission(BuildResult result)

submission.CompleteResults(result);

_overallBuildSuccess = _overallBuildSuccess && (_buildSubmissions[result.SubmissionId].BuildResult.OverallResult == BuildResultCode.Success);

CheckSubmissionCompletenessAndRemove(submission);
}
}
Expand All @@ -2611,8 +2607,6 @@ private void ReportResultsToSubmission(GraphBuildResult result)
{
submission.CompleteResults(result);

_overallBuildSuccess &= submission.BuildResult.OverallResult == BuildResultCode.Success;

CheckSubmissionCompletenessAndRemove(submission);
}
}
Expand All @@ -2628,6 +2622,7 @@ private void CheckSubmissionCompletenessAndRemove(BuildSubmission submission)
// If the submission has completed or never started, remove it.
if (submission.IsCompleted || submission.BuildRequest == null)
{
_overallBuildSuccess &= (submission.BuildResult?.OverallResult == BuildResultCode.Success);
_buildSubmissions.Remove(submission.SubmissionId);

// Clear all cached SDKs for the submission
Expand All @@ -2648,6 +2643,7 @@ private void CheckSubmissionCompletenessAndRemove(GraphBuildSubmission submissio
// If the submission has completed or never started, remove it.
if (submission.IsCompleted || !submission.IsStarted)
{
_overallBuildSuccess &= submission.BuildResult?.OverallResult == BuildResultCode.Success;
_graphBuildSubmissions.Remove(submission.SubmissionId);

// Clear all cached SDKs for the submission
Expand Down

0 comments on commit c8c2aaa

Please sign in to comment.