0

I have an application that connects to a REST API using async methods. I have this set up using async/await pretty much everywhere that connects to the API, however I have a question and some strange behavior that I don't completely understand. What I want to do is simply return a license in certain scenarios when the program shuts down. This is initiated by a window closing event; the event handler is as follows:

async void Window_Closing(object sender, System.ComponentModel.CancelEventArgs e)
        {
            ...other synchronous code...

            //Check for floating licensing
            if (KMApplication.License != null && KMApplication.License.Scope != Enums.LicenseScope.Standalone)
            {
                for (int i = 0; i < 3; i++)
                {
                    try
                    {

                        await KMApplication.License.ShutDown(KMApplication.Settings == null
                                                                 ? Enums.LicenseReturnModes.PromptOnShutdown
                                                                 : KMApplication.Settings.LicenseReturnMode)
                                           .ConfigureAwait(false);
                        break;
                    }
                    catch (Exception ex)
                    {
                        _logger.Warn("Exception in license release, attempt " + i, ex);
                    }
                }
            }

            await KMApplication.ApiService.Disconnect().ConfigureAwait(false);

            _logger.Info("Shutdown Complete");

            Application.Current?.Shutdown();
        }

When this runs I can step through in the debugger and it gets to the first license shutdown call which is the first async awaited call. Then when I press F10 to step to the next line of code it just shuts down and is gone. I verified that the license release that is supposed to be happening in that line is in face happening so it appears to run to completion of that line but then shuts down or crashes or something. I also looked at the logs and it never gets to the Shutdown Complete line and I don't believe it's getting to the ApiService.Disconnect either.

I also tried running this as a sync method using Task.Run(() => ...the method...).GetAwaiter().GetResult() but that just deadlocks on the first call.

How do I handle this and have it run the async release, wait for it to be done, then shut down?

sfaust
  • 2,089
  • 28
  • 54
  • 2
    `async void` won't wait for the method to complete because it's `void` and not `Task`. Of course, for events in WinForms, you can't use `async Task`. Likely scenario: the method is fired, and then your application closes (because it doesn't wait). You'll probably need to delay shutdown while this completes. – ProgrammingLlama Oct 16 '19 at 05:40
  • yeah since it's event handler I can't make it `async Task` because it won't match the signature which is what I'm struggling with. My real confusion is that it does actually shut down but the shutdown is called after the log method which never gets called... – sfaust Oct 16 '19 at 05:44
  • 1
    Async flow on a shutdown sounds weird. When the first async is initiated, the UI thread gets back to the message pump handling and effectively closes the window. – Dmytro Mukalov Oct 16 '19 at 05:52
  • 3
    Have you tried canceling the event (via the `CancelEventArgs`)? Your handler explicitly closes the app anyway. You may need to set a flag to ensure you don't get into an infinite loop. – John Wu Oct 16 '19 at 05:52
  • I hadn't tried canceling the event but I just did and no change. I agree that async on shutdown sounds a little strange but I don't know how else to go about it. I want the program to return the license when the user shuts down, and that's an async method since it's an http call to do it... – sfaust Oct 16 '19 at 06:01
  • what if you run without ConfigureAwait(false) or run it as `KMApplication.Licens....GetAwaiter().GetResult()` (without Task.Run) which will block synchronously. – user1672994 Oct 16 '19 at 06:10
  • Yes, that will deadlock, at least in this case... – sfaust Oct 16 '19 at 06:46
  • When the user tries to close the window, what is the desired behavior? Should the window close immediately and the finalizing work run in the backgroound without UI, or should the window delay closing until the finalizing work is complete? – Theodor Zoulias Oct 16 '19 at 11:38
  • Answered below, but the window should go away immediately (signaling to the user that the program is 'done') and then it should finish doing this final cleanup in the background before it's fully shut down. It should only take a couple seconds so it wouldn't be a big deal if it does stay open but it's better if it goes away right away which is what I have as far as I understand it. – sfaust Oct 16 '19 at 15:21
  • Did you know that he application simply runs a loop in the Main() method, and closing the window interrupts this loop, allowing the program to exit? If you put code after the loop, you can achieve the behavior synchronously. – theMayer Oct 17 '19 at 04:26

3 Answers3

2

The fundamental problem in what you're trying to do is that async/await assumes the main application thread continues running. This assumption directly conflicts with the shutdown action, whose job is to terminate all running tasks.

If you examine the documentation on Window_Closing, it states the following (and only the following):

Occurs directly after Close() is called, and can be handled to cancel window closure.

This is important. The only thing this is supposed to do is allow you to programmatically cancel the window closure, thus prompting some additional user action.

Your expectations are befuddled because of how async/await works. Async/await appears to run in a linear fashion; however, what actually happens is that control is passed back to the caller at the first await. The framework assumes at that point that you do not wish to cancel the form close, and the program is allowed to terminate, taking all other actions with it.

Fundamentally, all C-style programs have a main entry point, which runs a loop. It's been that way since the early days of C, and continues that way in WPF. However, in WPF, Microsoft got a bit clever, and decided to hide this from the programmer. There are a couple of options to deal with things that need to happen after main window closing:

  1. Re-hijack the main loop from your program, and put the code there. The details on how to do this may be found here.

  2. Set an explicit shutdown mode, and kick off the task to initiate that. Call Application.Shutdown() as the very last line of code you need to execute.

theMayer
  • 15,456
  • 7
  • 58
  • 90
  • Thanks but #2 is essentially what I already came up with with the help of Theodor in my answer... Upvoted for the detailed explanation though, thank you. – sfaust Oct 17 '19 at 14:12
  • There are pros and cons with your approach. Pros: by letting the window close before calling the async task, you have one less thing to worry about. Like timers kicking in the hidden window and changing the internal state. Cons: there is no way back. If the async task fails, you can't cancel the closing of the window because it has already closed. – Theodor Zoulias Oct 17 '19 at 14:39
  • @TheodorZoulias - you seem to not understand how the framework was designed to work, and particularly how this event handler was designed to work. The event handler is supposed to be synchronous. Either the form is allowed to close or it isn't, and that should be determinable **immediately.** Holding the form open via the event handler is not correct. If there is a desire to show a status back to the user, then you (1) cancel the form close, (2) kick off the background task which (3) displays some type of notice and (4) closes the form when it's finished. – theMayer Oct 17 '19 at 15:21
  • What I am comparing is an async version of the `FormClosing` event, with your approach of firing the async task after the closing of the form. I understand that you can't use the built-in synchronous `FormClosing` and expect it to work. This was actually the original problem of the OP, that motivated him to post this question. I have provided an implementation of `FormClosingAsync` for windows forms in [my answer](https://stackoverflow.com/a/58418905/11178549). – Theodor Zoulias Oct 17 '19 at 15:30
  • Arguing with you is useless. You *made up* an asynchronous operation where there isn’t one. That doesn’t work. I don’t know why that’s hard to get, but that’s where we are. – theMayer Oct 17 '19 at 17:12
  • I can assure you that the solution I suggested does work. If you don't want to consider it as an alternative to your solution, it's OK, I won't insist. – Theodor Zoulias Oct 17 '19 at 17:43
1

Ok here is what I ended up doing. Basically the window closing kicks off a task that will wait for the release to happen and then invoke the shutdown. This is what I was trying to do before but it didn't seem to work in async void method but it seems to be when done this way. Here is the new handler:

void Window_Closing(object sender, System.ComponentModel.CancelEventArgs e)
    {
        ...other sync code...

        Task.Run(async () =>
        {
            await InvokeKmShutdown();
            (Dispatcher ?? Dispatcher.CurrentDispatcher).InvokeShutdown();
        });
    }

And the shutdown method looks like this:

async Task InvokeKmShutdown()
    {
        ...other sync code...

        await KMApplication.ApiService.Disconnect();

        //Check for floating licensing
        if (KMApplication.License != null && KMApplication.License.Scope != License.Core.Enums.LicenseScope.Standalone)
        {
            for (int i = 0; i < 3; i++)
            {
                try
                {

                    await KMApplication.License.ShutDown(KMApplication.Settings == null
                                                             ? Enums.LicenseReturnModes.PromptOnShutdown
                                                             : KMApplication.Settings.LicenseReturnMode);
                    break;
                }
                catch (Exception ex)
                {
                    _logger.Warn("Exception in license release, attempt " + i, ex);
                }
            }
        }
    }

Hope it helps someone.

EDIT

Note that this is with an WPF app set to ShutdownMode="OnExplicitShutdown" in App.xaml so it won't shut down the actual app until I call the shutdown. If you are using WinForms or WPF is set to shut down on last window or main window close (main window close is the default I believe) you will end up with the race condition described in the comments below and may get the threads shut down before things run to completion.

sfaust
  • 2,089
  • 28
  • 54
  • Now you have a race condition. When the application exits will abort all background threads that are still running, and the thread-pool threads are background threads. By calling `Task.Run(async...)` you ofload the finalizing work to a thread-pool thread. You don't `Wait`/`await` the task, so it's a matter of luck if your finalizer will complete before the application calls `Abort` to the thread that runs the finalizer. – Theodor Zoulias Oct 16 '19 at 11:33
  • Understood but I don't think so here... This is the main window shutdown handler and since it isn't canceled the window will disappear which will keep the user from doing anything else while the task is running. Essentially this shutdown task should be the last thing running and since the method is awaited inside the task it will wait for that to complete before actually calling application shutdown. At that point there shouldn't be any background threads to worry about I don't think... Let me know if I'm off base somewhere. – sfaust Oct 16 '19 at 15:19
  • 1
    I assume that you open your form with `Application.Run(new Form1())`. Here is what the [documentation](https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.application.run?view=netframework-4.8#System_Windows_Forms_Application_Run_System_Windows_Forms_Form_) says about it: *This method adds an event handler to the mainForm parameter for the `Closed` event. The event handler calls `ExitThread` to clean up the application.* – Theodor Zoulias Oct 16 '19 at 15:48
  • It's not winforms, it's WPF. App.xaml has `StartupUri="MainWindow.xaml"` and `ShutdownMode="OnExplicitShutdown"` so no it shouldn't shut down unless I explicitly shut it down. I also verified this by commenting out the application shut down line of code and the window disappears but it remains debugging until I stop the debugger and never shuts down (I un-commented the line after verifying). – sfaust Oct 16 '19 at 17:37
  • 1
    That's a good point though. I just edited my answer to clarify that WPF needs to be set to explicit shutdown or you do get that race condition. – sfaust Oct 16 '19 at 17:41
  • 1
    You should probably edit your question to include the `wpf` tag as well... – Heretic Monkey Oct 16 '19 at 17:44
1

Here is an async version of the FormClosing event. It delays the closing of the form until the completion of the supplied Task. The user is prevented from closing the form before the completion of the task.

The OnFormClosingAsync event passes an enhanced version of the FormClosingEventArgs class to the handling code, with two additional properties: bool HideForm and int Timeout. These properties are read/write, much like the existing Cancel property. Setting HideForm to true has the effect of hiding the form while the async operation is in progress, to avoid frustrating the user. Setting Timeout to a value > 0 has the effect of abandoning the async operation after the specified duration in msec, and closing the form. Otherwise it is possible that the application could be left running indefinitely with a hidden UI, which could certainly be a problem. The Cancel property is still usable, and can be set to true by the handler of the event, to prevent the form from closing.

static class WindowsFormsAsyncExtensions
{
    public static IDisposable OnFormClosingAsync(this Form form,
        Func<object, FormClosingAsyncEventArgs, Task> handler)
    {
        Task compositeTask = null;
        form.FormClosing += OnFormClosing; // Subscribe to the event
        return new Disposer(() => form.FormClosing -= OnFormClosing);

        async void OnFormClosing(object sender, FormClosingEventArgs e)
        {
            if (compositeTask != null)
            {
                // Prevent the form from closing before the task is completed
                if (!compositeTask.IsCompleted) { e.Cancel = true; return; }
                // In case of success allow the form to close
                if (compositeTask.Status == TaskStatus.RanToCompletion) return;
                // Otherwise retry calling the handler
            }
            e.Cancel = true; // Cancel the normal closing of the form
            var asyncArgs = new FormClosingAsyncEventArgs(e.CloseReason);
            var handlerTask = await Task.Factory.StartNew(
                () => handler(sender, asyncArgs),
                CancellationToken.None, TaskCreationOptions.DenyChildAttach,
                TaskScheduler.Default); // Start in a thread-pool thread
            var hideForm = asyncArgs.HideForm;
            var timeout = asyncArgs.Timeout;
            if (hideForm) form.Visible = false;
            compositeTask = Task.WhenAny(handlerTask, Task.Delay(timeout)).Unwrap();
            try
            {
                await compositeTask; // Await and then continue in the UI thread
            }
            catch (OperationCanceledException) // Treat this as Cancel = true
            {
                if (hideForm) form.Visible = true;
                return;
            }
            catch // On error don't leave the form hidden
            {
                if (hideForm) form.Visible = true;
                throw;
            }
            if (asyncArgs.Cancel) // The caller requested to cancel the form close
            {
                compositeTask = null; // Forget the completed task
                if (hideForm) form.Visible = true;
                return;
            }
            await Task.Yield(); // Ensure that form.Close will run asynchronously
            form.Close(); // Finally close the form
        }
    }

    private struct Disposer : IDisposable
    {
        private readonly Action _action;
        public Disposer(Action disposeAction) => _action = disposeAction;
        void IDisposable.Dispose() => _action?.Invoke();
    }
}

public class FormClosingAsyncEventArgs : EventArgs
{
    public CloseReason CloseReason { get; }
    private volatile bool _cancel;
    public bool Cancel { get => _cancel; set => _cancel = value; }
    private volatile bool _hideForm;
    public bool HideForm { get => _hideForm; set => _hideForm = value; }
    private volatile int _timeout;
    public int Timeout { get => _timeout; set => _timeout = value; }

    public FormClosingAsyncEventArgs(CloseReason closeReason) : base()
    {
        this.CloseReason = closeReason;
        this.Timeout = System.Threading.Timeout.Infinite;
    }
}

Since OnFormClosingAsync is an extension method and not a real event, it can only have a single handler.

Usage example:

public Form1()
{
    InitializeComponent();
    this.OnFormClosingAsync(Window_FormClosingAsync);
}

async Task Window_FormClosingAsync(object sender, FormClosingAsyncEventArgs e)
{
    e.HideForm = true; // Optional
    e.Timeout = 5000; // Optional
    await KMApplication.License.ShutDown();
    //e.Cancel = true; // Optional
}

The Window_FormClosingAsync handler will run in a thread-pool thread, so it should not include any UI manipulation code.

Unsubscribing from the event is possible, by keeping a reference of the IDisposable return value, and disposing it.


Update: After reading this answer, I realized that it is possible to add a real event FormClosingAsync in the form, without creating a class that inherits from the form. This can be achieved by adding the event, and then running an initialization method that hooks the event to the native FormClosing event. Something like this:

public event Func<object, FormClosingAsyncEventArgs, Task> FormClosingAsync;

public Form1()
{
    InitializeComponent();
    this.InitFormClosingAsync(() => FormClosingAsync);

    this.FormClosingAsync += Window_FormClosingAsync_A;
    this.FormClosingAsync += Window_FormClosingAsync_B;
}

Inside the initializer, in the internal handler of the native FormClosing event, all the subscribers of the event can be retrieved using the GetInvocationList method:

var eventDelegate = handlerGetter();
if (eventDelegate == null) return;
var invocationList = eventDelegate.GetInvocationList()
    .Cast<Func<object, FormClosingAsyncEventArgs, Task>>().ToArray();

...and then invoked appropriately. All this adds complexity, while the usefulness of allowing multiple handlers is debated. So I would probably stick with the original single-handler design.


Update: It is still possible to have multiple handlers using the original method OnFormClosingAsync. It is quite easy actually. The Func<T> class inherits from Delegate, so it has invocation list like a real event:

Func<object, FormClosingAsyncEventArgs, Task> aggregator = null;
aggregator += Window_FormClosingAsync_A;
aggregator += Window_FormClosingAsync_B;
this.OnFormClosingAsync(aggregator);

No modification in the OnFormClosingAsync method is required.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
  • Awesome thank you. I will see if I can adapt the concept to WPF but I don't think it should be that hard. Accepting as this is a more thorough answer than mine. – sfaust Oct 16 '19 at 17:51
  • @sfaust my pleasure! I hope that there are no bugs in there, because there are a lot of scenarios covered, and I am not a great fan of unit testing. :-) – Theodor Zoulias Oct 16 '19 at 17:54
  • I removed a call to `.Result`, that could cause deadlocks. – Theodor Zoulias Oct 16 '19 at 22:35
  • The name I have chosen for the event (`FormClosingAsync`) may not be very appropriate. The Async suffix designates members that are asynchronous, i.e. they return a `Task` or another awaitable type. This event does not cover that criteria. Maybe "AsyncAware" would be more accurate, because it accepts Task-returning handlers, but this is too long for a suffix. – Theodor Zoulias Oct 18 '19 at 07:31