Fix build race condition in VS templates by lauren-ciha · Pull Request #6413 · microsoft/WindowsAppSDK

lauren-ciha · 2026-04-17T20:58:22Z

Before

If a user attempts to build the C# templates before the NuGet wizard installs package dependencies, the build would fail with Xaml compile errors

With this PR

If a user attempts to build the C# templates before the NuGet wizard installs:
- The build is blocked until the packages are installed
- An InfoBar appears notifying users that the build will continue once NuGet dependencies have installed
If automatic package download is disabled, an InfoBar appears alerting users that they will need to install packages before building the template

Validation

Manual Validation

Language	Packaged	Unit Test App	Wapproj	Class Library (C#) / Runtime Component (C++)
Criteria (Package auto-download enabled)	Build blocks with InfoBar message until download succeeds, then builds successfully (C#), builds as expected (C++)	Build blocks with InfoBar message until download succeeds, then builds successfully (C#), builds as expected (C++)	Build blocks with InfoBar message until download succeeds, then builds successfully (C#), builds as expected (C++)	Build blocks with InfoBar message until download succeeds, then builds successfully (C#), builds as expected (C++)
C#	Pass	Pass	Pass	Pass
C++	Pass	Pass	Pass	Pass
Criteria (Package auto-download disabled)	Warning InfoBar appears with instructions for downloading packages	Warning InfoBar appears with instructions for downloading packages	Warning InfoBar appears with instructions for downloading packages	Warning InfoBar appears with instructions for downloading packages
C#	Pass	Pass	Pass	Pass
C++	Pass	Pass	Pass	Pass

A microsoft employee must use /azp run to validate using the pipelines below.

WARNING:
Comments made by azure-pipelines bot maybe inaccurate.
Please see pipeline link to verify that the build is being ran.

For status checks on the main branch, please use TransportPackage-Foundation-PR
(https://microsoft.visualstudio.com/ProjectReunion/_build?definitionId=81063&_a=summary)
and run the build against your PR branch with the default parameters.

…r a number of manual tests, works with single-project C# and C#/C++ wapproj (not C# unit test)

…t templates

…nd start after package installation

…arted and the Unadvise call was premature

…f VS

…o longer used

…ngs (resolves in one click instead of two)

guimafelipe

Overall, it was a bit harder to track what the build guard is doing. There were a group of booleans that were always changed and used to check what to do. I would advise to change the design a bit to be a proper state machine, with clear transitions between the state instead of tracking those 6 boolean values. What do you think?

guimafelipe · 2026-04-21T00:08:52Z

+        public void SetReleaseCondition(Func<bool> condition)
+        {
+            _shouldRelease = condition;
+        }
+
+        public void SetInfoBarMessage(string message)
+        {
+            ThreadHelper.ThrowIfNotOnUIThread();
+            _infoBarMessage = message;
+            if (_isBlocking)
+            {
+                ShowInfoBarWhenShellReady();
+            }
+        }


Could we use the C# setter/getter here for clarity?

While it's theoretically possible, I'm not sure it makes sense for SetInfoBarMessage because it calls a UI change ShowInfoBarWhenShellIsReady() as well as assigning the _message value.

I'd add that a 'write-only' property is unorthodox - better a set method

guimafelipe · 2026-04-21T00:12:22Z

+                return;
+            }
+
+            _solutionBuildManager5 = ServiceProvider.GlobalProvider.GetService(typeof(SVsSolutionBuildManager)) as IVsSolutionBuildManager5;


Do we need to get it every time? Can we just set this up in the constructor once?

Scottj1s · 2026-05-14T20:07:41Z

+            }
+
+            _solutionBuildManager5 = ServiceProvider.GlobalProvider.GetService(typeof(SVsSolutionBuildManager)) as IVsSolutionBuildManager5;
+            if (_solutionBuildManager5 != null)


if it's not available, we just fall back to the race condition? probably fine, but a comment would help

Scottj1s · 2026-05-14T20:14:15Z

+            if (_shell == null)
+            {
+                _shell = ServiceProvider.GlobalProvider.GetService(typeof(SVsShell)) as IVsShell;
+            }
+
+            if (_shell == null)
+            {
+                return;
+            }


good use of an internal property, Shell

Scottj1s · 2026-05-14T20:23:16Z

+            _solutionBuildManager5 = ServiceProvider.GlobalProvider.GetService(typeof(SVsSolutionBuildManager)) as IVsSolutionBuildManager5;
+            if (_solutionBuildManager5 != null)
+            {
+                _solutionBuildManager5.AdviseUpdateSolutionEvents4(this, out _adviseCookie4);


not checking return for error (here and other places)

Scottj1s · 2026-05-14T20:23:40Z

+            // Shell not yet initialized; defer until it is
+            if (!_isShellPropertyAdvised)
+            {
+                _shell.AdviseShellPropertyChanges(this, out _shellPropertyCookie);


also no error checking

Scottj1s · 2026-05-14T21:28:15Z

+                    s_installationComplete = false;
+                }
+
+                s_buildGuard.SetReleaseCondition(() => s_installationComplete);


referencing s_buildGuard here without condition, while its assignment was conditional

Scottj1s · 2026-05-14T22:09:09Z

+                s_buildGuard.SetInfoBarMessage(string.Format(Resources._1055, projectName, packageNames));
+            }
+
+            if (!_packageRestoreEnabled && _nuGetPackages != null && _nuGetPackages.Any())


AI-generated code isn't always very compact - can just reuse condition on L133

Scottj1s · 2026-05-14T22:10:42Z

@@ -96,36 +166,55 @@ await ThreadHelper.JoinableTaskFactory.RunAsync(async () =>
                await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
                int canceled = 0; // Initialize as not canceled


move closer to call site, L183

Scottj1s · 2026-05-14T22:19:45Z

+        {
+        }
+
+        public void Dispose()


this doesn't appear to be implementing the dispose pattern - e.g., there are no unmanaged resources being released. I'd change the name to something else to avoid confusion.

Scottj1s · 2026-05-14T22:28:33Z

+
+        // NuGet package restore is enabled by default, so only return false if we can confirm it is disabled in
+        // the user's config.
+        private static bool IsNuGetPackageRestoreEnabled()


should also consult the global override env var EnableNuGetPackageRestore:
https://learn.microsoft.com/en-us/nuget/reference/nuget-config-file#packagerestore-section

Scottj1s · 2026-05-14T22:30:58Z

+        public int OnAfterOpenProject(IVsHierarchy pHierarchy, int fAdded)
+        {
+            return VSConstants.S_OK;
+        }


use expression-bodied for compact one-liners

Suggested change

public int OnAfterOpenProject(IVsHierarchy pHierarchy, int fAdded)

{

return VSConstants.S_OK;

}

public int OnAfterOpenProject(IVsHierarchy pHierarchy, int fAdded) => VSConstants.S_OK;

lauren-ciha added 25 commits April 17, 2026 13:17

First attempt at adding a build guard while packages are installing

4c95ea5

Successful flow with Single-project, C# wapproj flow disabled all builds

32a069b

Adds a check within build button handler to release build guard, afte…

6f89e42

…r a number of manual tests, works with single-project C# and C#/C++ wapproj (not C# unit test)

Add test explorer as a potential build command source for C# unit tes…

1f9fc8d

…t templates

Add auto-download setting check for smooth exit if disabled

e0d6066

Add checks for if NuGetRestore is disabled

f247d50

Add PackageReference fallback for C# projects

c596799

Attempt to add an automatic package download not enabled dialog

65c354d

C++ template shows infobar when auto download disabled

23b44e0

Attempt to eliminate polling

9ea5faf

Move polling into its own function

40cd26e

Differentiate C++ message in InfoBar

7a150da

Update infobar messages for auto-download disabled case

2598c02

Fix BuildGuard logic so infobar shows when builds are attempted

7ae5b8d

Disable the build guard infobar when build starts

afb0a4d

Update BuildGuard infobar message to reflect that builds are paused a…

38bdd27

…nd start after package installation

Add header back to Designer files

9b8cc4c

Link designer files in .csproj for C# and C++ templates

5730d5a

Remove unused ShowAutomaticPackageDownloadNotEnabledErrorDialog() method

49bf795

Remove activity in OnAfterOpenProjects - builds are disabled in RunSt…

46d9733

…arted and the Unadvise call was premature

Remove SolutionBuildManager2 because we're targeting newer versions o…

7537758

…f VS

Remove IVsUpdateSolutionEvents2 and its interfaces because they are n…

20a5c92

…o longer used

Update instructions for auto-package instruction to enable both setti…

cf428ed

…ngs (resolves in one click instead of two)

Add project null check in ProjectFinishedGenerating

69d7cc8

Reset source.extension.vsixmanifests to GetVSIXVersion

28dc0ab

microsoft-github-policy-service Bot added the needs-triage label Apr 17, 2026

lauren-ciha marked this pull request as ready for review April 17, 2026 22:41

lauren-ciha requested review from Scottj1s and guimafelipe April 17, 2026 22:41

guimafelipe reviewed Apr 21, 2026

View reviewed changes

Comment thread dev/VSIX/Shared/BuildGuard.cs

guimafelipe requested changes Apr 21, 2026

View reviewed changes

Remove duplicate null check for _solutionBuildManager5

56da5e1

Scottj1s reviewed May 14, 2026

View reviewed changes

Scottj1s requested changes May 14, 2026

View reviewed changes

		@@ -96,36 +166,55 @@ await ThreadHelper.JoinableTaskFactory.RunAsync(async () =>
		await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
		int canceled = 0; // Initialize as not canceled

Conversation

lauren-ciha commented Apr 17, 2026

Before

With this PR

Validation

Uh oh!

Uh oh!

guimafelipe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants