dotnet / runtime

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

Allow setting long preshutdown time in Windows Services #30386

Open Misiu opened 5 years ago

Misiu commented 5 years ago

This blog post describes how we can create Windows Service using .NET Core Workers. Currently, I have several Windows Services that are responsible for converting documents. Because the process can take a long time to complete I added a "hacky" way to change SERVICE_PRESHUTDOWN_INFO using ChangeServiceConfig2 from advapi32.dll and I accept SERVICE_ACCEPT_PRESHUTDOWN command.

For more info take a look at this Stack Overflow answer.

Now creating windows services is possible using .NET Core so I'd like to ask to expose a way to configure preshutdown time without using such hacky ways.

nxtn commented 4 years ago

Proposed API

namespace System.ServiceProcess
{
    public class ServiceBase
    {
        public bool CanHandlePreshutdownEvent { get; set; }

        protected virtual void OnPreshutdown();
    }
}

(Similar to CanHandleSessionChangeEvent and OnSessionChange.)

The timeout value should be configured during installation thus it's not included here.

Misiu commented 4 years ago

@NextTurn and how will we be able to set the timeout? Currently, there is no such property in ServiceInstaller. Linked StackOverflow answer showed a hacky way of doing that.

My intention is to be able to delay shutdown until the current document is converted, so there should be an easy way to tell the service can handle pre-shutdown and set that timeout value.

Should I add another issue for ServiceInstaller? Or can it be added here?

nxtn commented 4 years ago

Installer-related classes are tracked by #29643. For now you'll have to make your own ServiceInstaller or copy from some existing codes.

Misiu commented 4 years ago

@NextTurn Ideally, this should be done together. Any chance this might get added in 5.0?

danmoseley commented 4 years ago

Is delaying shutdown something we want to make easier to do? We would not want to offer API top do something "unnatural".

As noted in pinned issue, 5.0 release branches soon : I do not think anything here would make it.

Misiu commented 4 years ago

@danmosemsft I disagree on that. delaying shutdown isn't something "unnatural". I think that calling pinvokes, creating flags, structs and checking for system compatibility is "unnatural". See linked Stack Overflow answer. The use case is very simple: allow service to finish its work. I've created multiple services that use this. All of them are working till today but without .NET Core support of preshutdown I'm not even thinking about moving them to .NET Core. I don't expect first-class support for preshutdown, or a project template with additional step asking to enable this, but at least an API we can use and that doesn't require DllImports. ServiceInstaller is tracked in https://github.com/dotnet/runtime/issues/29643, it's just a matter of adding a single property and checking is the service has CanHandlePreshutdownEvent set to true and setting the timeout during installation.

Please reconsider supporting this.

nxtn commented 4 years ago

This will work even without a ServiceInstaller. The default preshutdown timeout is 3 minutes which should be sufficient for a number of apps.

Misiu commented 4 years ago

@NextTurn ok, but we need the API you proposed to tell the system that our service supports that, right? In the past, I've set all the services preshutdown timeout to 10 minutes. This was a safe value for me because sometimes 3 minutes wasn't enough, but if the service will support enabling preshutdown I can rewrite the service to .NET Core and hopefully, the default timeout will be ok.

kjkrum commented 4 years ago

This is essential. Delaying shutdown when appropriate isn't just normal, it's semi-officially recommended.