[release/7.0-staging] Make WindowsServiceLifetime gracefully stop (#8…

…5656) * Make WindowsServiceLifetime gracefully stop (#83892) * Make WindowsServiceLifetime gracefully stop WindowsServiceLifetime was not waiting for ServiceBase to stop the service. As a result we would sometimes end the process before notifying service control manager that the service had stopped -- resulting in an error in the eventlog and sometimes a service restart. We also were permitting multiple calls to Stop to occur - through SCM callbacks, and through public API. We must not call SetServiceStatus again once the service is marked as stopped. * Alternate approach to ensuring we only ever set STATE_STOPPED once. * Avoid calling ServiceBase.Stop on stopped service I fixed double-calling STATE_STOPPED in ServiceBase, but this fix will not be present on .NETFramework. Workaround that by avoiding calling ServiceBase.Stop when the service has already been stopped by SCM. * Add tests for WindowsServiceLifetime These tests leverage RemoteExecutor to avoid creating a separate service assembly. * Respond to feedback and add more tests. This better integrates with the RemoteExecutor component as well, by hooking up the service process and fetching its handle. This gives us the correct logging and exitcode handling from RemoteExecutor. * Honor Cancellation in StopAsync * Fix bindingRedirects in RemoteExecutor * Use Async lambdas for service testing * Fix issue on Win7 where duplicate service descriptions are disallowed * Respond to feedback * Fix comment and add timeout * Fix test condition * Enable M.E.H.WindowsServices and S.SP.ServiceController for servicing * Make service wait on its state before stopping (#84447) * Fix WindowsService Tests where RemoteExecutor is unsupported * Enable MS.W.C for servicing * Reference latest Microsoft.Extensions.Logging.Abstractions This package has been serviced and we compile against the serviced version of its assemblies. None of the directly referenced projects have been serviced so our package doesn't restore the serviced versions. Lift up the dependency on Logging.Abstractions to ensure we reference the serviced package. --------- Co-authored-by: Vladimir Sadov <[email protected]>
dotnet · May 4, 2023 · c8ab0d3 · c8ab0d3
1 parent 47f52f1
commit c8ab0d3
Show file tree

Hide file tree

Showing 12 changed files with 690 additions and 41 deletions.
diff --git a/eng/testing/xunit/xunit.targets b/eng/testing/xunit/xunit.targets
@@ -6,6 +6,11 @@
                       Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'" />
   </ItemGroup>
 
+  <PropertyGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
+    <AutoGenerateBindingRedirects Condition="'$(AutoGenerateBindingRedirects)' == ''">true</AutoGenerateBindingRedirects>
+    <GenerateBindingRedirectsOutputType Condition="'$(GenerateBindingRedirectsOutputType)' == ''">true</GenerateBindingRedirectsOutputType>
+  </PropertyGroup>
+
   <!-- Run target (F5) support. -->
   <PropertyGroup>
     <RunWorkingDirectory Condition="'$(RunWorkingDirectory)' == ''">$(OutDir)</RunWorkingDirectory>

diff --git a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.QueryServiceStatusEx.cs b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.QueryServiceStatusEx.cs
@@ -0,0 +1,34 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.Win32.SafeHandles;
+using System;
+using System.Runtime.InteropServices;
+
+internal static partial class Interop
+{
+    internal static partial class Advapi32
+    {
+        [StructLayout(LayoutKind.Sequential)]
+        internal struct SERVICE_STATUS_PROCESS
+        {
+            public int dwServiceType;
+            public int dwCurrentState;
+            public int dwControlsAccepted;
+            public int dwWin32ExitCode;
+            public int dwServiceSpecificExitCode;
+            public int dwCheckPoint;
+            public int dwWaitHint;
+            public int dwProcessId;
+            public int dwServiceFlags;
+        }
+
+        private const int SC_STATUS_PROCESS_INFO = 0;
+
+        [LibraryImport(Libraries.Advapi32, SetLastError = true)]
+        [return: MarshalAs(UnmanagedType.Bool)]
+        private static unsafe partial bool QueryServiceStatusEx(SafeServiceHandle serviceHandle, int InfoLevel, SERVICE_STATUS_PROCESS* pStatus, int cbBufSize, out int pcbBytesNeeded);
+
+        internal static unsafe bool QueryServiceStatusEx(SafeServiceHandle serviceHandle, SERVICE_STATUS_PROCESS* pStatus) => QueryServiceStatusEx(serviceHandle, SC_STATUS_PROCESS_INFO, pStatus, sizeof(SERVICE_STATUS_PROCESS), out _);
+    }
+}
diff --git a/src/libraries/Common/src/Interop/Windows/Interop.Errors.cs b/src/libraries/Common/src/Interop/Windows/Interop.Errors.cs
@@ -64,6 +64,8 @@ internal static partial class Errors
         internal const int ERROR_IO_PENDING = 0x3E5;
         internal const int ERROR_NO_TOKEN = 0x3f0;
         internal const int ERROR_SERVICE_DOES_NOT_EXIST = 0x424;
+        internal const int ERROR_EXCEPTION_IN_SERVICE = 0x428;
+        internal const int ERROR_PROCESS_ABORTED = 0x42B;
         internal const int ERROR_NO_UNICODE_TRANSLATION = 0x459;
         internal const int ERROR_DLL_INIT_FAILED = 0x45A;
         internal const int ERROR_COUNTER_TIMEOUT = 0x461;

diff --git a/...xtensions.Hosting.WindowsServices/src/Microsoft.Extensions.Hosting.WindowsServices.csproj b/...xtensions.Hosting.WindowsServices/src/Microsoft.Extensions.Hosting.WindowsServices.csproj
@@ -7,6 +7,8 @@
     <EnableAOTAnalyzer>true</EnableAOTAnalyzer>
     <PackageDescription>.NET hosting infrastructure for Windows Services.</PackageDescription>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
+    <ServicingVersion>1</ServicingVersion>
   </PropertyGroup>
 
   <ItemGroup>
@@ -27,6 +29,7 @@
 
   <ItemGroup>
     <ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.Hosting\src\Microsoft.Extensions.Hosting.csproj" />
+    <ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.Logging.Abstractions\src\Microsoft.Extensions.Logging.Abstractions.csproj" />
     <ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.Logging.EventLog\src\Microsoft.Extensions.Logging.EventLog.csproj" />
   </ItemGroup>
 

diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
@@ -15,8 +15,10 @@ namespace Microsoft.Extensions.Hosting.WindowsServices
     public class WindowsServiceLifetime : ServiceBase, IHostLifetime
     {
         private readonly TaskCompletionSource<object?> _delayStart = new TaskCompletionSource<object?>(TaskCreationOptions.RunContinuationsAsynchronously);
+        private readonly TaskCompletionSource<object?> _serviceDispatcherStopped = new TaskCompletionSource<object?>(TaskCreationOptions.RunContinuationsAsynchronously);
         private readonly ManualResetEventSlim _delayStop = new ManualResetEventSlim();
         private readonly HostOptions _hostOptions;
+        private bool _serviceStopRequested;
 
         public WindowsServiceLifetime(IHostEnvironment environment, IHostApplicationLifetime applicationLifetime, ILoggerFactory loggerFactory, IOptions<HostOptions> optionsAccessor)
             : this(environment, applicationLifetime, loggerFactory, optionsAccessor, Options.Options.Create(new WindowsServiceLifetimeOptions()))
@@ -69,19 +71,30 @@ private void Run()
             {
                 Run(this); // This blocks until the service is stopped.
                 _delayStart.TrySetException(new InvalidOperationException("Stopped without starting"));
+                _serviceDispatcherStopped.TrySetResult(null);
             }
             catch (Exception ex)
             {
                 _delayStart.TrySetException(ex);
+                _serviceDispatcherStopped.TrySetException(ex);
             }
         }
 
-        public Task StopAsync(CancellationToken cancellationToken)
+        /// <summary>
+        /// Called from <see cref="IHost.StopAsync"/> to stop the service if not already stopped, and wait for the service dispatcher to exit.
+        /// Once this method returns the service is stopped and the process can be terminated at any time.
+        /// </summary>
+        public async Task StopAsync(CancellationToken cancellationToken)
         {
-            // Avoid deadlock where host waits for StopAsync before firing ApplicationStopped,
-            // and Stop waits for ApplicationStopped.
-            Task.Run(Stop, CancellationToken.None);
-            return Task.CompletedTask;
+            cancellationToken.ThrowIfCancellationRequested();
+
+            if (!_serviceStopRequested)
+            {
+                await Task.Run(Stop, cancellationToken).ConfigureAwait(false);
+            }
+
+            // When the underlying service is stopped this will cause the ServiceBase.Run method to complete and return, which completes _serviceDispatcherStopped.
+            await _serviceDispatcherStopped.Task.ConfigureAwait(false);
         }
 
         // Called by base.Run when the service is ready to start.
@@ -91,18 +104,28 @@ protected override void OnStart(string[] args)
             base.OnStart(args);
         }
 
-        // Called by base.Stop. This may be called multiple times by service Stop, ApplicationStopping, and StopAsync.
-        // That's OK because StopApplication uses a CancellationTokenSource and prevents any recursion.
+        /// <summary>
+        /// Executes when a Stop command is sent to the service by the Service Control Manager (SCM).
+        /// Triggers <see cref="IHostApplicationLifetime.ApplicationStopping"/> and waits for <see cref="IHostApplicationLifetime.ApplicationStopped"/>.
+        /// Shortly after this method returns, the Service will be marked as stopped in SCM and the process may exit at any point.
+        /// </summary>
         protected override void OnStop()
         {
+            _serviceStopRequested = true;
             ApplicationLifetime.StopApplication();
             // Wait for the host to shutdown before marking service as stopped.
             _delayStop.Wait(_hostOptions.ShutdownTimeout);
             base.OnStop();
         }
 
+        /// <summary>
+        /// Executes when a Shutdown command is sent to the service by the Service Control Manager (SCM).
+        /// Triggers <see cref="IHostApplicationLifetime.ApplicationStopping"/> and waits for <see cref="IHostApplicationLifetime.ApplicationStopped"/>.
+        /// Shortly after this method returns, the Service will be marked as stopped in SCM and the process may exit at any point.
+        /// </summary>
         protected override void OnShutdown()
         {
+            _serviceStopRequested = true;
             ApplicationLifetime.StopApplication();
             // Wait for the host to shutdown before marking service as stopped.
             _delayStop.Wait(_hostOptions.ShutdownTimeout);

diff --git a/...s.Hosting.WindowsServices/tests/Microsoft.Extensions.Hosting.WindowsServices.Tests.csproj b/...s.Hosting.WindowsServices/tests/Microsoft.Extensions.Hosting.WindowsServices.Tests.csproj
@@ -4,12 +4,45 @@
     <!-- Use "$(NetCoreAppCurrent)-windows" to avoid PlatformNotSupportedExceptions from ServiceController. -->
     <TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetFrameworkMinimum)</TargetFrameworks> 
     <EnableDefaultItems>true</EnableDefaultItems>
+    <EnableLibraryImportGenerator>true</EnableLibraryImportGenerator>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <IncludeRemoteExecutor>true</IncludeRemoteExecutor>
   </PropertyGroup>
 
   <ItemGroup>
     <ProjectReference Include="..\src\Microsoft.Extensions.Hosting.WindowsServices.csproj" />
   </ItemGroup>
 
+  <ItemGroup>
+    <Compile Include="$(LibrariesProjectRoot)System.ServiceProcess.ServiceController\src\Microsoft\Win32\SafeHandles\SafeServiceHandle.cs"
+             Link="Microsoft\Win32\SafeHandles\SafeServiceHandle.cs" />
+    <Compile Include="$(CommonPath)DisableRuntimeMarshalling.cs"
+             Link="Common\DisableRuntimeMarshalling.cs"
+             Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'" />
+    <Compile Include="$(CommonPath)Interop\Windows\Interop.Errors.cs"
+             Link="Common\Interop\Windows\Interop.Errors.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Interop.Libraries.cs"
+             Link="Common\Interop\Windows\Interop.Libraries.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.ServiceProcessOptions.cs"
+             Link="Common\Interop\Windows\Interop.ServiceProcessOptions.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.CloseServiceHandle.cs"
+             Link="Common\Interop\Windows\Interop.CloseServiceHandle.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.CreateService.cs"
+             Link="Common\Interop\Windows\Interop.CreateService.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.DeleteService.cs"
+             Link="Common\Interop\Windows\Interop.DeleteService.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.OpenService.cs"
+             Link="Common\Interop\Windows\Interop.OpenService.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.OpenSCManager.cs"
+             Link="Common\Interop\Windows\Interop.OpenSCManager.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.QueryServiceStatus.cs"
+             Link="Common\Interop\Windows\Interop.QueryServiceStatus.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.QueryServiceStatusEx.cs"
+             Link="Common\Interop\Windows\Interop.QueryServiceStatusEx.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.SERVICE_STATUS.cs"
+             Link="Common\Interop\Windows\Interop.SERVICE_STATUS.cs" />
+  </ItemGroup>
+
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
     <Reference Include="System.ServiceProcess" />
   </ItemGroup>

diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs
@@ -2,9 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
-using System.IO;
 using System.Reflection;
 using System.ServiceProcess;
+using Microsoft.DotNet.RemoteExecutor;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting.Internal;
 using Microsoft.Extensions.Hosting.WindowsServices;
@@ -17,6 +17,8 @@ namespace Microsoft.Extensions.Hosting
 {
     public class UseWindowsServiceTests
     {
+        private static bool IsRemoteExecutorSupportedAndPrivilegedProcess => RemoteExecutor.IsSupported && AdminHelpers.IsProcessElevated();
+
         private static MethodInfo? _addWindowsServiceLifetimeMethod = null;
 
         [Fact]
@@ -30,6 +32,26 @@ public void DefaultsToOffOutsideOfService()
             Assert.IsType<ConsoleLifetime>(lifetime);
         }
 
+        [ConditionalFact(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))]
+        public void CanCreateService()
+        {
+            using var serviceTester = WindowsServiceTester.Create(() =>
+            {
+                using IHost host = new HostBuilder()
+                    .UseWindowsService()
+                    .Build();
+                host.Run();
+            });
+
+            serviceTester.Start();
+            serviceTester.WaitForStatus(ServiceControllerStatus.Running);
+            serviceTester.Stop();
+            serviceTester.WaitForStatus(ServiceControllerStatus.Stopped);
+
+            var status = serviceTester.QueryServiceStatus();
+            Assert.Equal(0, status.win32ExitCode);
+        }
+
         [Fact]
         public void ServiceCollectionExtensionMethodDefaultsToOffOutsideOfService()
         {
@@ -66,7 +88,7 @@ public void ServiceCollectionExtensionMethodSetsEventLogSourceNameToApplicationN
             var builder = new HostApplicationBuilder(new HostApplicationBuilderSettings
             {
                 ApplicationName = appName,
-            }); 
+            });
 
             // Emulate calling builder.Services.AddWindowsService() from inside a Windows service.
             AddWindowsServiceLifetime(builder.Services);
@@ -82,7 +104,7 @@ public void ServiceCollectionExtensionMethodSetsEventLogSourceNameToApplicationN
         [Fact]
         public void ServiceCollectionExtensionMethodCanBeCalledOnDefaultConfiguration()
         {
-            var builder = new HostApplicationBuilder(); 
+            var builder = new HostApplicationBuilder();
 
             // Emulate calling builder.Services.AddWindowsService() from inside a Windows service.
             AddWindowsServiceLifetime(builder.Services);