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

Implements RequestAdditionalTime for use in pending states #72

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Implements RequestAdditionalTime for use in pending states #72

wants to merge 2 commits into from

Conversation

cerebrate
Copy link

Implements the equivalent of ServiceBase.RequestAdditionalTime for services which are slow to transition between states.

(It does this as a method of Win32ServiceHost, which requires keeping a reference to Win32ServiceHost around where the service implementation can call it; that's not the cleanest possible way of doing it, but adding it to IWin32Service/IPausableWin32Service would be a breaking change.)

@turowicz
Copy link

Please merge this @dasMulli

@dasMulli
Copy link
Owner

This should be implemented in an IWin32ServiceStateMachine subclass which is responsible for calling the ServiceStatusReportCallback callback with appropriate commands.

Exposing the control over something that the state machine is supposed to handle feels too hacky.

Right now, this can already be implemented in user code by e.g. copying SimpleServiceStateMachine to user code and adding timeout handling logic there.

There are a few things that could be done here:

  1. Add a new interface for services that has additional callback methods on the Start/Stop/* methods - IWin32Service2 or ITimeoutAwareWin32Service etc.
  2. Add a new extending interface that can be checked for at run-time, e.g. ITimeoutAwareWin32Service { TimeoutCallback { set; } } that requires a setter for a timeout callback,
    • Add an state machine that already knows how to expand the timeout while waiting for user code.

I'm actually quite interested in doing (3) and adding something like an IAsyncWin32Service with async start/stop/* methods instead. The calling state machine would then be able to do a timeout-based wait on the task - requesting additional time for each wait.

@turowicz
Copy link

@dasMulli could you release an alpha package with above PR at least?

@dasMulli
Copy link
Owner

@dasMulli
Copy link
Owner

@turowicz I'd be interested to know if your problem is mainly requiring additional time for the stop operation or more in general? the default timeout could also be made configurable for all operations of the service host..

@turowicz
Copy link

Hey yes I need to protect my service from exiting too early. The time it requires to quit is variable, depending on which task it's currently processing.

@turowicz
Copy link

... but that said I think I will just upgrade to dotnet core 2.1-rc1 and use ServiceBase.

@cerebrate
Copy link
Author

@dasMulli Well, (1) was the first approach that occurred to me, but thinking about it from a library perspective rather than one specific to my usage, it seemed to me that this would lead to a rather inelegant profusion of IVariantWin32Service interfaces and associated state machines.

(For example, another thing I'm working on - and would like to contribute back - is an implementation of the Shutdown and Power Event service functionalities. Even if we assume that for most purposes Shutdown is a de facto power event, my specific requirement is for a service that can also request more time - it has a variable length shutdown - and pause/continue, so short of turning my specific requirements into a new interface/state machine and leaving it at that, we end up with IPausableWin32Service, IPowerAwareWin32Service, ITimeoutAwareWin32Service, and all the various combinations of same to let users have the features they want. Still thinking about the best approach here.)

And so I wanted something that could operate with any existing and potential future state machine. It is uncomfortably hacky, though, so if you'll leave this pull request open for a couple of days, I'll look into refactoring it to something more along the lines of (2).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants