From 975830b0eef395a97f1665efba367ed94979b8b9 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Thu, 7 Oct 2021 15:27:35 -0500 Subject: [PATCH 1/2] Check BackgroundService.ExecuteTask for null In some scenarios, derived classes might not have called base.StartAsync, and ExecuteTask will still be null. Ensure we don't fail in those cases. Fix #60131 --- .../src/Internal/Host.cs | 33 +++++++++------- .../tests/UnitTests/Internal/HostTests.cs | 39 +++++++++++++++++++ 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs b/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs index 4041133a0a9f25..c60d5fc16d8125 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs @@ -79,24 +79,29 @@ public async Task StartAsync(CancellationToken cancellationToken = default) private async Task TryExecuteBackgroundServiceAsync(BackgroundService backgroundService) { - try + // backgroundService.ExecuteTask may not be set (e.g. if the derived class doesn't call base.StartAsync) + Task backgroundTask = backgroundService.ExecuteTask; + if (backgroundTask != null) { - await backgroundService.ExecuteTask.ConfigureAwait(false); - } - catch (Exception ex) - { - // When the host is being stopped, it cancels the background services. - // This isn't an error condition, so don't log it as an error. - if (_stopCalled && backgroundService.ExecuteTask.IsCanceled && ex is OperationCanceledException) + try { - return; + await backgroundTask.ConfigureAwait(false); } - - _logger.BackgroundServiceFaulted(ex); - if (_options.BackgroundServiceExceptionBehavior == BackgroundServiceExceptionBehavior.StopHost) + catch (Exception ex) { - _logger.BackgroundServiceStoppingHost(ex); - _applicationLifetime.StopApplication(); + // When the host is being stopped, it cancels the background services. + // This isn't an error condition, so don't log it as an error. + if (_stopCalled && backgroundTask.IsCanceled && ex is OperationCanceledException) + { + return; + } + + _logger.BackgroundServiceFaulted(ex); + if (_options.BackgroundServiceExceptionBehavior == BackgroundServiceExceptionBehavior.StopHost) + { + _logger.BackgroundServiceStoppingHost(ex); + _applicationLifetime.StopApplication(); + } } } } diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Internal/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Internal/HostTests.cs index d07c2bc57eaa71..60835d28665901 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Internal/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Internal/HostTests.cs @@ -1394,6 +1394,36 @@ public async Task HostNoErrorWhenServiceIsCanceledAsPartOfStop() } } + /// + /// Tests that when a BackgroundService does not call base, the Host still starts and stops successfully. + /// + [Fact] + public async Task StartOnBackgroundServiceThatDoesNotCallBase() + { + TestLoggerProvider logger = new TestLoggerProvider(); + + using IHost host = CreateBuilder() + .ConfigureLogging(logging => + { + logging.AddProvider(logger); + }) + .ConfigureServices(services => + { + services.AddHostedService(); + }) + .Build(); + + host.Start(); + await host.StopAsync(); + + foreach (LogEvent logEvent in logger.GetEvents()) + { + Assert.True(logEvent.LogLevel <= LogLevel.Information, "All logged events should be les than or equal to Information. No Warnings or Errors."); + + Assert.NotEqual("BackgroundServiceFaulted", logEvent.EventId.Name); + } + } + private IHostBuilder CreateBuilder(IConfiguration config = null) { return new HostBuilder().ConfigureHostConfiguration(builder => builder.AddConfiguration(config ?? new ConfigurationBuilder().Build())); @@ -1562,5 +1592,14 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) } } } + + private class BackgroundServiceDoesNotCallBase : BackgroundService + { + public override Task StartAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + protected override Task ExecuteAsync(CancellationToken stoppingToken) => Task.CompletedTask; + + public override Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + } } } From 07fe4ac8b64ee2cb43eaf1a6d16c94cc20584a48 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Thu, 7 Oct 2021 15:54:45 -0500 Subject: [PATCH 2/2] PR feedback --- .../src/Internal/Host.cs | 36 ++++++++++--------- .../tests/UnitTests/Internal/HostTests.cs | 2 +- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs b/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs index c60d5fc16d8125..2f73146e695ef2 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs @@ -81,27 +81,29 @@ private async Task TryExecuteBackgroundServiceAsync(BackgroundService background { // backgroundService.ExecuteTask may not be set (e.g. if the derived class doesn't call base.StartAsync) Task backgroundTask = backgroundService.ExecuteTask; - if (backgroundTask != null) + if (backgroundTask == null) { - try + return; + } + + try + { + await backgroundTask.ConfigureAwait(false); + } + catch (Exception ex) + { + // When the host is being stopped, it cancels the background services. + // This isn't an error condition, so don't log it as an error. + if (_stopCalled && backgroundTask.IsCanceled && ex is OperationCanceledException) { - await backgroundTask.ConfigureAwait(false); + return; } - catch (Exception ex) - { - // When the host is being stopped, it cancels the background services. - // This isn't an error condition, so don't log it as an error. - if (_stopCalled && backgroundTask.IsCanceled && ex is OperationCanceledException) - { - return; - } - _logger.BackgroundServiceFaulted(ex); - if (_options.BackgroundServiceExceptionBehavior == BackgroundServiceExceptionBehavior.StopHost) - { - _logger.BackgroundServiceStoppingHost(ex); - _applicationLifetime.StopApplication(); - } + _logger.BackgroundServiceFaulted(ex); + if (_options.BackgroundServiceExceptionBehavior == BackgroundServiceExceptionBehavior.StopHost) + { + _logger.BackgroundServiceStoppingHost(ex); + _applicationLifetime.StopApplication(); } } } diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Internal/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Internal/HostTests.cs index 60835d28665901..b3133dba3be886 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Internal/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Internal/HostTests.cs @@ -1418,7 +1418,7 @@ public async Task StartOnBackgroundServiceThatDoesNotCallBase() foreach (LogEvent logEvent in logger.GetEvents()) { - Assert.True(logEvent.LogLevel <= LogLevel.Information, "All logged events should be les than or equal to Information. No Warnings or Errors."); + Assert.True(logEvent.LogLevel <= LogLevel.Information, "All logged events should be less than or equal to Information. No Warnings or Errors."); Assert.NotEqual("BackgroundServiceFaulted", logEvent.EventId.Name); }