Skip to content

Commit

Permalink
[release/7.0-staging] Make WindowsServiceLifetime gracefully stop (#8…
Browse files Browse the repository at this point in the history
…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 <vsadov@microsoft.com>
  • Loading branch information
ericstj and VSadov committed May 4, 2023
1 parent 47f52f1 commit c8ab0d3
Show file tree
Hide file tree
Showing 12 changed files with 690 additions and 41 deletions.
5 changes: 5 additions & 0 deletions eng/testing/xunit/xunit.targets
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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 _);
}
}
2 changes: 2 additions & 0 deletions src/libraries/Common/src/Interop/Windows/Interop.Errors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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]
Expand All @@ -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()
{
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Loading

0 comments on commit c8ab0d3

Please sign in to comment.