Fix CI flakiness: async race conditions, disposal crash, and stdout truncation

azure-pipelines-pr.yml

@@ -78,6 +78,7 @@ stages:
           matrix:
             Build_Release:
               _BuildConfig: Release
+              _ExtraBuildArgs: ''
               # PRs or external builds are not signed.
               ${{ if or(eq(variables['System.TeamProject'], 'public'), in(variables['Build.Reason'], 'PullRequest')) }}:
                 _SignType: test
@@ -87,6 +88,10 @@ stages:
               Build_Debug:
                 _BuildConfig: Debug
                 _SignType: test
+                # Disable publish for Debug to avoid concurrent uploads to the same
+                # PackageArtifacts/BlobArtifacts containers as the Release build,
+                # which causes "Blob is incomplete" artifact corruption.
+                _ExtraBuildArgs: '/p:Publish=false'
 
         steps:
         - checkout: self
@@ -98,6 +103,7 @@ stages:
             -prepareMachine
             -integrationTest
             $(_InternalBuildArgs)
+            $(_ExtraBuildArgs)
           displayName: Windows Build / Publish
 
       - ${{ if or(eq(variables['System.TeamProject'], 'public'), in(variables['Build.Reason'], 'PullRequest')) }}:
@@ -109,14 +115,20 @@ stages:
               debug_configuration:
                 _BuildConfig: Debug
                 _SignType: none
+                # Disable publish for Debug to avoid concurrent uploads to the same
+                # PackageArtifacts/BlobArtifacts containers as the Release build,
+                # which causes "Blob is incomplete" artifact corruption.
+                _ExtraBuildArgs: '/p:Publish=false'
               release_configuration:
                 _BuildConfig: Release
                 _SignType: none
+                _ExtraBuildArgs: ''
           steps:
           - script: eng/common/cibuild.sh
               --configuration $(_BuildConfig)
               --prepareMachine
               --integrationTest
+              $(_ExtraBuildArgs)
             name: Build
             displayName: Build
 
@@ -133,14 +145,20 @@ stages:
               debug_configuration:
                 _BuildConfig: Debug
                 _SignType: none
+                # Disable publish for Debug to avoid concurrent uploads to the same
+                # PackageArtifacts/BlobArtifacts containers as the Release build,
+                # which causes "Blob is incomplete" artifact corruption.
+                _ExtraBuildArgs: '/p:Publish=false'
               release_configuration:
                 _BuildConfig: Release
                 _SignType: none
+                _ExtraBuildArgs: ''
           steps:
           - script: eng/common/cibuild.sh
               --configuration $(_BuildConfig)
               --prepareMachine
               --integrationTest
+              $(_ExtraBuildArgs)
             name: Build
             displayName: Build
             condition: succeeded()

src/Microsoft.TemplateEngine.Edge/BuiltInManagedProvider/GlobalSettings.cs

@@ -56,8 +56,8 @@ public void Dispose()
             {
                 return;
             }
-            _watcher?.Dispose();
             _disposed = true;
+            _watcher?.Dispose();
             _watcher = null;
         }
 
@@ -167,6 +167,13 @@ public async Task SetInstalledTemplatePackagesAsync(IReadOnlyList<TemplatePackag
         //  To prevent this - we try to wait for a lock on behalf of the handler and refuse all concurrent file change notifications in the meantime
         private async void FileChanged(object sender, FileSystemEventArgs e)
         {
+            // FileSystemWatcher fires callbacks on threadpool threads that can race with Dispose().
+            // This pre-lock check handles the common case where the callback fires after _disposed is set.
+            if (_disposed)
+            {
+                return;
+            }
+
             // Make sure the waiting happens only for one notification at the time - as we do not care about other notifications
             // until the SettingsChanged is called
             //  if multiple concurrent call(s) get here, while there is already other caller inside waiting for the lock
@@ -178,6 +185,14 @@ private async void FileChanged(object sender, FileSystemEventArgs e)
 
             await TryWaitForLock().ConfigureAwait(false);
 
+            // Re-check after lock wait: the object may have been disposed while we were waiting
+            // for the lock. Without this guard, SettingsChanged subscribers would call back into
+            // disposed state. Stress testing confirms this fires in ~99% of disposal-during-callback races.
+            if (_disposed)
+            {
+                return;
+            }
+
             // We are ready for new notifications now - indicate so by clearing the counter
             Interlocked.Exchange(ref _waitingInstances, 0);
 

src/Microsoft.TemplateEngine.Edge/BuiltInManagedProvider/GlobalSettingsTemplatePackageProvider.cs

@@ -16,6 +16,7 @@ internal class GlobalSettingsTemplatePackageProvider : IManagedTemplatePackagePr
         private readonly Dictionary<Guid, IInstaller> _installersByGuid = new Dictionary<Guid, IInstaller>();
         private readonly Dictionary<string, IInstaller> _installersByName = new Dictionary<string, IInstaller>();
         private readonly GlobalSettings _globalSettings;
+        private volatile bool _disposed;
 
         public GlobalSettingsTemplatePackageProvider(GlobalSettingsTemplatePackageProviderFactory factory, IEngineEnvironmentSettings settings)
         {
@@ -43,7 +44,7 @@ public GlobalSettingsTemplatePackageProvider(GlobalSettingsTemplatePackageProvid
             string globalSettingsFilePath = Path.Combine(environmentSettings.Paths.GlobalSettingsDir, "packages.json");
             _globalSettings = new GlobalSettings(environmentSettings, globalSettingsFilePath);
             // We can't just add "SettingsChanged+=TemplatePackagesChanged", because TemplatePackagesChanged is null at this time.
-            _globalSettings.SettingsChanged += () => TemplatePackagesChanged?.Invoke();
+            _globalSettings.SettingsChanged += OnGlobalSettingsChanged;
         }
 
         public event Action? TemplatePackagesChanged;
@@ -197,9 +198,22 @@ public async Task<IReadOnlyList<UpdateResult>> UpdateAsync(IEnumerable<UpdateReq
 
         public void Dispose()
         {
+            _disposed = true;
+            _globalSettings.SettingsChanged -= OnGlobalSettingsChanged;
             _globalSettings.Dispose();
         }
 
+        private void OnGlobalSettingsChanged()
+        {
+            // Guard against SettingsChanged firing during cascading disposal: Dispose() sets _disposed
+            // then unsubscribes, but an in-flight callback may already be past the delegate check.
+            if (_disposed)
+            {
+                return;
+            }
+            TemplatePackagesChanged?.Invoke();
+        }
+
         private async Task UpdateTemplatePackagesMetadataAsync(IEnumerable<IManagedTemplatePackage> templatePackages, CancellationToken cancellationToken)
         {
             _ = templatePackages ?? throw new ArgumentNullException(nameof(templatePackages));

src/Microsoft.TemplateEngine.Edge/Settings/TemplatePackageManager.cs

@@ -22,6 +22,7 @@ public class TemplatePackageManager : IDisposable
         private readonly Scanner _installScanner;
         private volatile TemplateCache? _userTemplateCache;
         private Dictionary<ITemplatePackageProvider, Task<IReadOnlyList<ITemplatePackage>>>? _cachedSources;
+        private volatile bool _disposed;
 
         /// <summary>
         /// Creates the instance.
@@ -116,6 +117,7 @@ public async Task<IReadOnlyList<ITemplatePackage>> GetTemplatePackagesAsync(bool
 
         public void Dispose()
         {
+            _disposed = true;
             if (_cachedSources == null)
             {
                 return;
@@ -255,6 +257,12 @@ private void EnsureProvidersLoaded()
             {
                 provider.TemplatePackagesChanged += () =>
                 {
+                    // Events from providers may be in-flight when Dispose is called. Guard against
+                    // updating _cachedSources or raising TemplatePackagesChanged on a disposed instance.
+                    if (_disposed)
+                    {
+                        return;
+                    }
                     _cachedSources[provider] = provider.GetAllTemplatePackagesAsync(default);
                     TemplatePackagesChanged?.Invoke();
                 };
@@ -342,7 +350,7 @@ private async Task<TemplateCache> UpdateTemplateCacheAsync(bool needsRebuild, Ca
             }
 
             var scanResults = new ScanResult?[allTemplatePackages.Count];
-            Parallel.For(0, allTemplatePackages.Count, async (index) =>
+            await Task.WhenAll(Enumerable.Range(0, allTemplatePackages.Count).Select(async index =>
             {
                 try
                 {
@@ -354,7 +362,7 @@ private async Task<TemplateCache> UpdateTemplateCacheAsync(bool needsRebuild, Ca
                     _logger.LogWarning(LocalizableStrings.TemplatePackageManager_Error_FailedToScan, allTemplatePackages[index].MountPointUri, ex.Message);
                     _logger.LogDebug($"Stack trace: {ex.StackTrace}");
                 }
-            });
+            })).ConfigureAwait(false);
             cancellationToken.ThrowIfCancellationRequested();
             cache = new TemplateCache(allTemplatePackages, scanResults, mountPoints, _environmentSettings);
             foreach (var scanResult in scanResults)

test/Microsoft.TemplateEngine.Edge.UnitTests/GlobalSettingsTests.cs

@@ -51,7 +51,19 @@ public async Task TestFileWatcher()
             using var globalSettings1 = new GlobalSettings(envSettings, settingsFile);
             using var globalSettings2 = new GlobalSettings(envSettings, settingsFile);
             var taskSource = new TaskCompletionSource<TemplatePackageData>();
-            globalSettings2.SettingsChanged += async () => taskSource.TrySetResult((await globalSettings2.GetInstalledTemplatePackagesAsync(default)).Single());
+            globalSettings2.SettingsChanged += () =>
+            {
+                try
+                {
+                    var result = globalSettings2.GetInstalledTemplatePackagesAsync(default).GetAwaiter().GetResult();
+                    taskSource.TrySetResult(result.Single());
+                }
+                catch (ObjectDisposedException)
+                {
+                    // FileSystemWatcher callbacks race with test cleanup disposal.
+                    // This handler may fire after globalSettings2 is disposed at end of test.
+                }
+            };
             var mutex = await globalSettings1.LockAsync(default);
             var newData = new TemplatePackageData(
                 Guid.NewGuid(),
@@ -60,7 +72,7 @@ public async Task TestFileWatcher()
                 new Dictionary<string, string>() { { "a", "b" } });
             await globalSettings1.SetInstalledTemplatePackagesAsync(new[] { newData }, default);
             mutex.Dispose();
-            var timeoutTask = Task.Delay(2000);
+            var timeoutTask = Task.Delay(10000);
             var firstFinishedTask = await Task.WhenAny(timeoutTask, taskSource.Task);
             Assert.Equal(taskSource.Task, firstFinishedTask);
 

tools/Microsoft.TemplateEngine.Authoring.CLI/Commands/ExecutableCommand.cs

@@ -39,11 +39,19 @@ private sealed class CommandAction : AsynchronousCommandLineAction
 
             public override async Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
             {
-                using ILoggerFactory loggerFactory = LoggerFactory.Create(builder => builder.AddSimpleConsole(c => c.ColorBehavior = LoggerColorBehavior.Disabled));
-                TModel arguments = _command.ParseContext(parseResult);
-
-                //exceptions are handled by parser itself
-                return await _command.ExecuteAsync(arguments, loggerFactory, cancellationToken).ConfigureAwait(false);
+                int result;
+                using (ILoggerFactory loggerFactory = LoggerFactory.Create(builder => builder.AddSimpleConsole(c => c.ColorBehavior = LoggerColorBehavior.Disabled)))
+                {
+                    TModel arguments = _command.ParseContext(parseResult);
+
+                    //exceptions are handled by parser itself
+                    result = await _command.ExecuteAsync(arguments, loggerFactory, cancellationToken).ConfigureAwait(false);
+                }
+                // Ensure all console output is flushed after the logger factory
+                // (and its background queue thread) has been disposed.
+                Console.Out.Flush();
+                Console.Error.Flush();
+                return result;
             }
         }
     }