diff --git a/azure-pipelines-pr.yml b/azure-pipelines-pr.yml index 02d66751e56..a6cbd3030ec 100644 --- a/azure-pipelines-pr.yml +++ b/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() diff --git a/src/Microsoft.TemplateEngine.Edge/BuiltInManagedProvider/GlobalSettings.cs b/src/Microsoft.TemplateEngine.Edge/BuiltInManagedProvider/GlobalSettings.cs index 33832329c1e..22796ba26d0 100644 --- a/src/Microsoft.TemplateEngine.Edge/BuiltInManagedProvider/GlobalSettings.cs +++ b/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 _installersByGuid = new Dictionary(); private readonly Dictionary _installersByName = new Dictionary(); 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> UpdateAsync(IEnumerable templatePackages, CancellationToken cancellationToken) { _ = templatePackages ?? throw new ArgumentNullException(nameof(templatePackages)); diff --git a/src/Microsoft.TemplateEngine.Edge/Settings/TemplatePackageManager.cs b/src/Microsoft.TemplateEngine.Edge/Settings/TemplatePackageManager.cs index 0229c3790be..df730356f1a 100644 --- a/src/Microsoft.TemplateEngine.Edge/Settings/TemplatePackageManager.cs +++ b/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>>? _cachedSources; + private volatile bool _disposed; /// /// Creates the instance. @@ -116,6 +117,7 @@ public async Task> 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 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 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) diff --git a/test/Microsoft.TemplateEngine.Edge.UnitTests/GlobalSettingsTests.cs b/test/Microsoft.TemplateEngine.Edge.UnitTests/GlobalSettingsTests.cs index e7ff04c8f8c..0b31cda0d91 100644 --- a/test/Microsoft.TemplateEngine.Edge.UnitTests/GlobalSettingsTests.cs +++ b/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(); - 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() { { "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); diff --git a/tools/Microsoft.TemplateEngine.Authoring.CLI/Commands/ExecutableCommand.cs b/tools/Microsoft.TemplateEngine.Authoring.CLI/Commands/ExecutableCommand.cs index 529aca1b110..e7dbf2e18f4 100644 --- a/tools/Microsoft.TemplateEngine.Authoring.CLI/Commands/ExecutableCommand.cs +++ b/tools/Microsoft.TemplateEngine.Authoring.CLI/Commands/ExecutableCommand.cs @@ -39,11 +39,19 @@ private sealed class CommandAction : AsynchronousCommandLineAction public override async Task 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; } } }