Make WindowsServiceLifetime gracefully stop #83892
Conversation
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.
|
Tagging subscribers to this area: @dotnet/area-system-serviceprocess Issue DetailsWindowsServiceLifetime 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. Fixes: #62579
|
src/libraries/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceBase.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
Outdated
Show resolved
Hide resolved
Yeah but needs some work as we need a service project. Plan to do so before moving out of draft. |
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.
These tests leverage RemoteExecutor to avoid creating a separate service assembly.
ericstj
force-pushed
the
fixServiceLifetime
branch
from
10c5529 to
eb39960
Compare
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.QueryServiceStatusEx.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
Outdated
Show resolved
Hide resolved
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.
|
These tests are failing in CI but passing locally: |
|
So the netfx tests are failing because they aren't getting bindingRedirects in AzDo build. Binlog shows the following parameters are missing from call to RAR: - AppConfigFile = C:\src\dotnet\runtime\eng\testing\netfx.exe.config
- SupportsBindingRedirectGeneration = TrueSeems significant. |
|
So it looks like this difference is due to the following. In CI builds we don't reference Microsoft.NET.Test.Sdk: runtime/eng/testing/xunit/xunit.props Line 26 in ef6b6d5 The Microsoft.NET.Test.Sdk sets Arcade sets the property, but does so after RAR, so it doesn't help (I have no idea why it's done after RAR): So nothing is telling RAR to find and generate bindingRedirects in our test projects. So that raises the question -- how are these loads succeeding for our test projects in helix today? The answer is they aren't, initially: What happens is Xunit has installed an AssemblyResolve handler that probes the test directory and ignores assembly version completely: So this will break anything in remote-executor scenarios that requires bindingRedirects, since they won't be using that runner to set up the AssemblyResolve handler -- they need the bindingRedirects to be correct. |
src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/WindowsServiceLifetimeTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
Outdated
Show resolved
Hide resolved
|
JIT test failure is #84398. |
|
/backport to release/7.0-staging |
|
/backport to release/6.0-staging |
|
Started backporting to release/7.0-staging: https://github.com/dotnet/runtime/actions/runs/4625519941 |
|
Started backporting to release/6.0-staging: https://github.com/dotnet/runtime/actions/runs/4625520553 |
|
@ericstj backporting to release/7.0-staging failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Make WindowsServiceLifetime gracefully stop
Applying: Alternate approach to ensuring we only ever set STATE_STOPPED once.
Applying: Avoid calling ServiceBase.Stop on stopped service
error: sha1 information is lacking or useless (src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 Avoid calling ServiceBase.Stop on stopped service
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@ericstj an error occurred while backporting to release/7.0-staging, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
|
@ericstj backporting to release/6.0-staging failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Make WindowsServiceLifetime gracefully stop
Using index info to reconstruct a base tree...
M src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
M src/libraries/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceBase.cs
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceBase.cs
Auto-merging src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
CONFLICT (content): Merge conflict in src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Make WindowsServiceLifetime gracefully stop
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@ericstj an error occurred while backporting to release/6.0-staging, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
* 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
* 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
…5661) * 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 # Conflicts: # src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs * 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 # Conflicts: # src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs * Port changes to 6.0 codebase * Version Microsoft.Windows.Compatibility --------- Co-authored-by: Vladimir Sadov <[email protected]>
…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]>
ghost
locked as resolved and limited conversation to collaborators
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.
Fixes: #62579