Graceful stop of worker services

[danielmarbach]

Proposed Changes

As discussed in #771 this makes sure any work that is left is executed and stop only exits after the worker has exited.

The reintroduces the necessary evil GetAwaiter().GetResult() on the highest entry level possible to make the underlying call stack async all the way.

The caveats of this have been discussed in the mentioned PR and seemed to have been outweighed against the benefits of finalizing the work.

[danielmarbach]

In additional to that it would be possible to introduce

either

            public void Enqueue(Work work)
            {
                if (_tokenSource.IsCancellationRequested)
                {
                    return;
                }
                _workQueue.Enqueue(work);
                _syncSource.TrySetResult(true);
            }

or

            public void Enqueue(Work work)
            {
                _tokenSource.Token.ThrowIfCancellationRequested();
                _workQueue.Enqueue(work);
                _syncSource.TrySetResult(true);
            }

thoughts?

[stebet]

Considering that stoppage has been explicitly requested, I'd think the non-throwing options would make more sense, since you probably don't care about any new work being added at that point?

[danielmarbach]

That was also my idea but I didn't want to bias this discussion too much up front

[michaelklishin]

I also think the non-throwing version makes more sense.

[danielmarbach]

One thing though:

Given that this check is in the hotpath and cannot be optimized away do we really want to do this?

[michaelklishin]

That's a good point. I think as long as the previously enqueued work items are processed, we are reasonably safe.

[lukebakken]

I'll review this after #768 / #779