From 07fdbb7bc32e97db80649e762189c6378aaa8656 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:21:55 +0000 Subject: [PATCH 1/3] Initial plan From 02eb3484ed61b91a2cf75d2bf20d1646c6ca76a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:30:53 +0000 Subject: [PATCH 2/3] Fix stale request from clearing new request's active target entry Co-authored-by: AR-May <67507805+AR-May@users.noreply.github.com> --- .../RequestBuilder/TargetBuilder.cs | 8 +++---- .../Shared/BuildRequestConfiguration.cs | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs index ad694052f21..1d1e96741a0 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs @@ -177,7 +177,7 @@ public async Task BuildTargets(ProjectLoggingContext loggingContext // If there are still targets left on the stack, they need to be removed from the 'active targets' list foreach (TargetEntry target in _targetsToBuild) { - configuration.ActivelyBuildingTargets.Remove(target.Name); + configuration.RemoveActivelyBuildingTargetIfOwnedBy(target.Name, _requestEntry.Request.GlobalRequestId); } ((IBuildComponent)taskBuilder).ShutdownComponent(); @@ -505,7 +505,7 @@ await PushTargets(errorTargets, currentTargetEntry, currentTargetEntry.Lookup, t } catch { - _requestEntry.RequestConfiguration.ActivelyBuildingTargets.Remove(currentTargetEntry.Name); + _requestEntry.RequestConfiguration.RemoveActivelyBuildingTargetIfOwnedBy(currentTargetEntry.Name, _requestEntry.Request.GlobalRequestId); throw; } } @@ -527,7 +527,7 @@ await PushTargets(errorTargets, currentTargetEntry, currentTargetEntry.Lookup, t } // This target is no longer actively building. - _requestEntry.RequestConfiguration.ActivelyBuildingTargets.Remove(currentTargetEntry.Name); + _requestEntry.RequestConfiguration.RemoveActivelyBuildingTargetIfOwnedBy(currentTargetEntry.Name, _requestEntry.Request.GlobalRequestId); _buildResult.AddResultsForTarget(currentTargetEntry.Name, targetResult); @@ -628,7 +628,7 @@ private void PopDependencyTargetsOnTargetFailure(TargetEntry topEntry, TargetRes entry.LeaveLegacyCallTargetScopes(); // This target is no longer actively building (if it was). - _requestEntry.RequestConfiguration.ActivelyBuildingTargets.Remove(topEntry.Name); + _requestEntry.RequestConfiguration.RemoveActivelyBuildingTargetIfOwnedBy(topEntry.Name, _requestEntry.Request.GlobalRequestId); // If we come across an entry which requires us to stop processing (for instance, an aftertarget of the original // CallTarget target) then we need to use that flag, not the one from the top entry. diff --git a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs index 5ed219b6fde..24d07287657 100644 --- a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs +++ b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs @@ -644,6 +644,27 @@ public Lookup BaseLookup public Dictionary ActivelyBuildingTargets => _activelyBuildingTargets ?? (_activelyBuildingTargets = new Dictionary(StringComparer.OrdinalIgnoreCase)); + /// + /// Removes from , but only if it is still + /// recorded as being built by . This guards against a stale request (for + /// instance one whose cancellation timed out but which is still executing) resuming after its entry was + /// overwritten -- or the table was cleared and repopulated -- by a newer request reusing this retained + /// configuration. Without this check, the stale request could erroneously remove the newer request's + /// still-active entry, which could in turn cause the configuration's to be + /// cached while the newer request is still using it. + /// + /// The name of the target that is no longer actively building for the calling request. + /// The global request id of the request which believes it owns the target entry. + internal void RemoveActivelyBuildingTargetIfOwnedBy(string targetName, int globalRequestId) + { + if (_activelyBuildingTargets is not null && + _activelyBuildingTargets.TryGetValue(targetName, out int owningRequestId) && + owningRequestId == globalRequestId) + { + _activelyBuildingTargets.Remove(targetName); + } + } + /// /// Keeps the in memory while the caller uses it, preventing a concurrent /// memory-pressure cache sweep. Retrieves the project first if it was already cached. From 317402180dbb560b45cf8877b5940b9553e87d6f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:34:46 +0000 Subject: [PATCH 3/3] Add unit test for guarded ActivelyBuildingTargets removal Co-authored-by: AR-May <67507805+AR-May@users.noreply.github.com> --- .../BuildRequestConfiguration_Tests.cs | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs b/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs index 1f66abcb633..ff5b47f7fdd 100644 --- a/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs +++ b/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs @@ -776,5 +776,41 @@ public void TestProjectEvaluationIdPreservedAcrossTranslateForFutureUse() deserialized.ProjectEvaluationId.ShouldBe(expectedEvalId); } + + /// + /// Verifies that only removes the + /// entry when it is still owned by the specified request id. This protects against a stale request (e.g. one + /// whose cancellation timed out but which is still executing) resuming and erroneously clearing a different, + /// newer request's still-active entry for the same target name and configuration. + /// + [Fact] + public void RemoveActivelyBuildingTargetIfOwnedByOnlyRemovesMatchingOwner() + { + BuildRequestData data = new BuildRequestData("file", new Dictionary(), "toolsVersion", Array.Empty(), null); + BuildRequestConfiguration configuration = new BuildRequestConfiguration(1, data, "2.0"); + + const int staleRequestId = 1; + const int newRequestId = 2; + + // The stale request originally recorded that it is building "Build". + configuration.ActivelyBuildingTargets["Build"] = staleRequestId; + + // A newer request reused this retained configuration and is now building "Build" instead. + configuration.ActivelyBuildingTargets["Build"] = newRequestId; + + // The stale request finally resumes (e.g. after a cancellation timeout) and tries to mark its target as + // no longer building. Since it no longer owns the entry, this must be a no-op. + configuration.RemoveActivelyBuildingTargetIfOwnedBy("Build", staleRequestId); + + configuration.ActivelyBuildingTargets.ShouldContainKey("Build"); + configuration.ActivelyBuildingTargets["Build"].ShouldBe(newRequestId); + configuration.IsActivelyBuilding.ShouldBeTrue(); + + // The owning (new) request completes normally and removes its own entry. + configuration.RemoveActivelyBuildingTargetIfOwnedBy("Build", newRequestId); + + configuration.ActivelyBuildingTargets.ShouldNotContainKey("Build"); + configuration.IsActivelyBuilding.ShouldBeFalse(); + } } }