dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.84k stars 4.62k forks source link

UseWindowsService always reports successful service startup even when Host startup fails #50019

Open davidmatson opened 4 years ago

davidmatson commented 4 years ago

Describe the bug

When using .UseWindowsService(), the windows service always reports success from startup, regardless of whether the host actually starts up successfully. A clear and concise description of what the bug is.

To Reproduce

  1. Use the code below.
  2. Start the service with net start.

Expected behavior

Net start indicates the service failed to start.

Actual behavior

Net start indicates the service started successfully.

Additional context

Note that on some older versions of .NET Framework (pre 4.7.2 maybe), service startup exceptions aren't propagated correctly.

Program.cs:

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using System;
using System.Threading;
using System.Threading.Tasks;

static class Program
{
    static void Main()
    {
        new HostBuilder()
            .ConfigureLogging(l => l.AddConsole())
            .ConfigureServices((s) =>
            {
                s.AddHostedService<TestService>();
            })
            .UseWindowsService()
            .Build()
            .Run();
    }
}

class TestService : IHostedService
{
    readonly ILogger log;

    public TestService(ILogger<TestService> log)
    {
        this.log = log;
    }

    public Task StartAsync(CancellationToken cancellationToken)
    {
        log.LogInformation("Starting...");
        throw new InvalidOperationException("Test when startup always fails.");
        //log.LogInformation("Started.");
        //return Task.CompletedTask;
    }

    public Task StopAsync(CancellationToken cancellationToken)
    {
        log.LogInformation("Stopping...");
        log.LogInformation("Stopped.");
        return Task.CompletedTask;
    }
}

TestHostBuilder.csproj:


<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
  <PropertyGroup>
    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
    <ProjectGuid>{9463B6C5-BC95-43AA-968F-651084C85DAE}</ProjectGuid>
    <OutputType>Exe</OutputType>
    <RootNamespace>TestHostBuilder</RootNamespace>
    <AssemblyName>TestHostBuilder</AssemblyName>
    <TargetFrameworkVersion>v4.8</TargetFrameworkVersion>
    <FileAlignment>512</FileAlignment>
    <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>
    <Deterministic>true</Deterministic>
  </PropertyGroup>
  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
    <PlatformTarget>AnyCPU</PlatformTarget>
    <DebugSymbols>true</DebugSymbols>
    <DebugType>full</DebugType>
    <Optimize>false</Optimize>
    <OutputPath>bin\Debug\</OutputPath>
    <DefineConstants>DEBUG;TRACE</DefineConstants>
    <ErrorReport>prompt</ErrorReport>
    <WarningLevel>4</WarningLevel>
  </PropertyGroup>
  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
    <PlatformTarget>AnyCPU</PlatformTarget>
    <DebugType>pdbonly</DebugType>
    <Optimize>true</Optimize>
    <OutputPath>bin\Release\</OutputPath>
    <DefineConstants>TRACE</DefineConstants>
    <ErrorReport>prompt</ErrorReport>
    <WarningLevel>4</WarningLevel>
  </PropertyGroup>
  <ItemGroup>
    <Reference Include="System" />
    <Reference Include="System.Core" />
    <Reference Include="System.Xml.Linq" />
    <Reference Include="System.Data.DataSetExtensions" />
    <Reference Include="Microsoft.CSharp" />
    <Reference Include="System.Data" />
    <Reference Include="System.Net.Http" />
    <Reference Include="System.Xml" />
  </ItemGroup>
  <ItemGroup>
    <Compile Include="Program.cs" />
    <Compile Include="Properties\AssemblyInfo.cs" />
  </ItemGroup>
  <ItemGroup>
    <None Include="App.config" />
  </ItemGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Hosting">
      <Version>3.1.0</Version>
    </PackageReference>
    <PackageReference Include="Microsoft.Extensions.Hosting.WindowsServices">
      <Version>3.1.0</Version>
    </PackageReference>
  </ItemGroup>
  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
</Project>```
davidmatson commented 4 years ago

The problematic line of code appears to be: https://github.com/aspnet/Extensions/blob/b83b27d76439497459fe9cf7337d5128c900eb5a/src/Hosting/WindowsServices/src/WindowsServiceLifetime.cs#L91

The actual service startup logic only unblocks the app from starting - it doesn't wait for the app to start and throw an exception if startup fails.

If the OnStart method awaited a task that does actual host/app startup, that would likely fix the problem. (It would also fix the problem that the Windows Service is reporting as started before it is fully started, which is another related bug that currently exists.)

davidmatson commented 4 years ago

I think .UseWindowsService() may just not be possible to do correctly - the Service class and IHostLifetime abstractions just don't match. Particularly, the Service class expects all startup logic to live inside OnStart, and for the program not to do anything that might throw before then.

I tried ways to solve this bug as well as dotnet/runtime#50018. The only solution I found to both was not to use UseWindowsService at all. An IHostLifetime isn't told about exception starting up hosted services. The best it can do is know if the application has started via IHostApplicationLifetime, but it won't know when an exception in Internal\Host.cs's startup means that method will never get called. I even tried using AppDomain.CurrentDomain.UnhandledException, but that doesn't work all the time (only works if the program doesn't handle exceptions), and doesn't allow the service to report startup failure, only to have it terminate abnormally.

Here's what I ended up doing - use only WindowsServiceHelpers.IsWindowsService() and not .UseWindowsService(). Having all startup for a Windows services happen inside Service.OnStart is how that API is intended to work, and all these bugs go away when it's used that way.

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Hosting.WindowsServices;
using Microsoft.Extensions.Logging;
using System;
using System.ServiceProcess;
using System.Threading;
using System.Threading.Tasks;

class Program
{
    public static int Main()
    {
        // See the following issues for more information:
        // https://github.com/dotnet/extensions/issues/2831
        // https://github.com/dotnet/extensions/issues/2835
        // https://github.com/dotnet/extensions/issues/2836
        if (WindowsServiceHelpers.IsWindowsService())
        {
            RunViaWindowsService();
        }
        else
        {
            try
            {
                RunViaConsole();
            }
            catch (Exception exception)
            {
                Console.Error.WriteLine(exception);
                return exception.HResult;
            }
        }

        return 0;
    }

    static IHost CreateHost()
    {
        return new HostBuilder()
            .ConfigureLogging(l => l.AddConsole())
            .ConfigureServices((s) =>
            {
                s.AddHostedService<TestService>();
            })
            .Build();
    }

    static void RunViaConsole()
    {
        CreateHost()
            // This is a long-running process that only needs to be restarted if it crashes.
            // Effectively, this call starts a thread polling a queue and invoking callbacks in the Functions class
            // as-needed.
            .Run();
    }

    static void RunViaWindowsService()
    {
        ServiceBase.Run(new WindowsService(CreateHost));
    }

    class WindowsService : ServiceBase
    {
        readonly Func<IHost> createHost;

        IHost host;
        bool disposed;

        public WindowsService(Func<IHost> createHost)
        {
            if (createHost == null)
            {
                throw new ArgumentNullException(nameof(createHost));
            }

            this.createHost = createHost;
        }

        protected override void OnStart(string[] args)
        {
            host = CreateHost();
            host.StartAsync().GetAwaiter().GetResult();
            base.OnStart(args);
        }

        protected override void OnStop()
        {
            host.StopAsync().GetAwaiter().GetResult();
            base.OnStop();
        }

        protected override void Dispose(bool disposing)
        {
            if (disposing && !disposed)
            {
                host.Dispose();
                disposed = true;
            }
        }
    }

    class TestService : IHostedService
    {
        readonly ILogger log;

        public TestService(ILogger<TestService> log)
        {
            this.log = log;
        }

        public Task StartAsync(CancellationToken cancellationToken)
        {
            log.LogInformation("Starting...");
            throw new InvalidOperationException("Test when startup always fails.");
            //log.LogInformation("Started.");
            //return Task.CompletedTask;
        }

        public Task StopAsync(CancellationToken cancellationToken)
        {
            log.LogInformation("Stopping...");
            log.LogInformation("Stopped.");
            return Task.CompletedTask;
        }
    }
}
kmcclellan commented 4 years ago

Here's a workaround that still leverages the functionality of UseWindowsService() and WindowsServiceLifetime. Hoping this gets fixed at some point and I can remove this from my application!

    static class WindowsServiceAdapter
    {
        public static IHostBuilder UseWindowsService(this IHostBuilder builder)
        {
            WindowsServiceLifetimeHostBuilderExtensions.UseWindowsService(builder);

            return builder.ConfigureServices(services =>
            {
                var lifetime = services.FirstOrDefault(s => s.ImplementationType == typeof(WindowsServiceLifetime));
                if (lifetime != null)
                {
                    services.Remove(lifetime);
                    services.AddSingleton<IHostLifetime, Lifetime>();
                }
            });
        }

        private class Lifetime : WindowsServiceLifetime, IHostLifetime
        {
            private readonly CancellationTokenSource _starting = new CancellationTokenSource();
            private readonly ManualResetEventSlim _started = new ManualResetEventSlim();
            private readonly IHostApplicationLifetime _applicationLifetime;

            public Lifetime(
                IHostEnvironment environment,
                IHostApplicationLifetime applicationLifetime,
                ILoggerFactory loggerFactory,
                IOptions<HostOptions> optionsAccessor)
                : base(environment, applicationLifetime, loggerFactory, optionsAccessor)
            {
                _applicationLifetime = applicationLifetime;
            }

            public new async Task WaitForStartAsync(CancellationToken cancellationToken)
            {
                _applicationLifetime.ApplicationStarted.Register(() => _started.Set());

                try
                {
                    using var cts = CancellationTokenSource.CreateLinkedTokenSource(_starting.Token, cancellationToken);
                    await base.WaitForStartAsync(cts.Token);
                }
                catch (OperationCanceledException) when (_starting.IsCancellationRequested) { }
            }

            protected override void OnStart(string[] args)
            {
                _starting.Cancel();

                // Make sure the application actually started successfully.
                _started.Wait(_applicationLifetime.ApplicationStopping);
                if (!_applicationLifetime.ApplicationStarted.IsCancellationRequested)
                {
                    throw new Exception("Failed to start host");
                }

                base.OnStart(args);
            }

            protected override void Dispose(bool disposing)
            {
                if (disposing)
                {
                    _starting.Dispose();
                    _started.Set();
                }

                base.Dispose(disposing);
            }
        }
    }
dotnet-issue-labeler[bot] commented 3 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

ghost commented 3 years ago

Tagging subscribers to this area: @eerhardt, @maryamariyan See info in area-owners.md if you want to be subscribed.

Issue Details
### Describe the bug When using .UseWindowsService(), the windows service always reports success from startup, regardless of whether the host actually starts up successfully. A clear and concise description of what the bug is. ### To Reproduce 1. Use the code below. 2. Start the service with net start. ### Expected behavior Net start indicates the service failed to start. ### Actual behavior Net start indicates the service started successfully. ### Additional context Note that on some older versions of .NET Framework (pre 4.7.2 maybe), service startup exceptions aren't propagated correctly. Program.cs: ```c# using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using System; using System.Threading; using System.Threading.Tasks; static class Program { static void Main() { new HostBuilder() .ConfigureLogging(l => l.AddConsole()) .ConfigureServices((s) => { s.AddHostedService(); }) .UseWindowsService() .Build() .Run(); } } class TestService : IHostedService { readonly ILogger log; public TestService(ILogger log) { this.log = log; } public Task StartAsync(CancellationToken cancellationToken) { log.LogInformation("Starting..."); throw new InvalidOperationException("Test when startup always fails."); //log.LogInformation("Started."); //return Task.CompletedTask; } public Task StopAsync(CancellationToken cancellationToken) { log.LogInformation("Stopping..."); log.LogInformation("Stopped."); return Task.CompletedTask; } } ``` TestHostBuilder.csproj: ```c# Debug AnyCPU {9463B6C5-BC95-43AA-968F-651084C85DAE} Exe TestHostBuilder TestHostBuilder v4.8 512 true true AnyCPU true full false bin\Debug\ DEBUG;TRACE prompt 4 AnyCPU pdbonly true bin\Release\ TRACE prompt 4 3.1.0 3.1.0 ```
Author: davidmatson
Assignees: -
Labels: `area-Extensions-Hosting`, `untriaged`
Milestone: -