Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/7.0-staging] Make WindowsServiceLifetime gracefully stop #85656

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the main PR I found no description for this change except "Fixing binding redirects for remote excutor". For my own education, would you mind explaining this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We weren't generating any bindingRedirects for tests, but they need them.

When running from XUnit we were working because XUnit has an AssemblyResolve hook that does a similar thing (though it can't do the exact same thing).

When I added out-of-proc services with RemoteExecutor -- really any use of remote executor -- loads failed since they didn't have this resolve handler. Ensuring the app.config is generated makes sure that remote executor gets it. It will also avoid us needing to rely on the AssemblyResolve hook in Xunit (and it's fragility).

Here's the discussion: #83892 (comment)

<!-- 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" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What code in WindowsServiceLifetime.cs caused the need to add this new project reference?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this is the transitive dependency problem you described here, right? #85656 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. It's a side-effect of incremental servicing and our package testing caught it. Good reason to pay attention to those failures 👍

<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