How to create a robust WinForms app that uses async and DI

[MisinformedDNA]

I've been using async and DI for some time, but applying it to WinForms has been a struggle.

Let's start with DI. Forms are registered with a transient lifetime. By default, all child forms will be created in the root scope and, thus, won't be garbage collected until the app closes. That's a serious memory leak. To get around this, I'm creating a scope first:

// RootForm.cs
private void OpenScopedChildMdiForm<T>() where T : Form
{
    var scope = _serviceProvider.CreateScope();
    var form = scope.ServiceProvider.GetRequiredService<T>();

    form.FormClosed += (_, _) => scope.Dispose();

    form.MdiParent = this;
    form.Show();
}

When the form is closed, the scope is closed, which in turn, starts disposing all the objects injected into the form, even if the form is still using them.

Also, if async operations are still in progress in the handlers, errors are going to occur. For that, I introduced a default error handler in my base form. I also realized that I should cancel existing operations, so I added a CancellationTokenSource to my base form as well.

public partial class MyForm : Form
{
    private readonly CancellationTokenSource _cancellationTokenSource = new();

    protected Form()
    {
        InitializeComponent();
    }

    public CancellationToken GetCancellationToken() => _cancellationTokenSource.Token;

    protected async Task ExecuteAsync(Func<Task> func)
    {
        try
        {
            await func();
        }
        catch (OperationCanceledException)
        {
            // Operation cancelled. OK to ignore exceptions.
        }
        catch (Exception) when (IsDisposed)
        {
            // Form is disposed. OK to ignore exceptions.
            // Often `ObjectDisposedException`, but not always.  
        }
    }

    protected override void OnFormClosing(FormClosingEventArgs e)
    {
        _cancellationTokenSource.Cancel();
        base.OnFormClosing(e);
    }
}

Handlers would then look like this:

private async void btnSave_Click(object sender, EventArgs e)
{
    await ExecuteAsync(async () => await OnSaveAsync(GetCancellationToken()));
}

Even with all this, there is still a memory leak. The form is not always GCed and seems more common if there are still async operations running. Closing and reopening the form causes linear memory growth.

I analyzed many dumps and the form is always somehow referenced by an EF Core context (and the scope referenced below is not the local variable that I'm creating either).

-> 00000170022A7598 Microsoft.EntityFrameworkCore.Infrastructure.CoreOptionsExtension
-> 00000170022A1610 Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope
-> 00000170022EA2C0 System.Collections.Generic.List`1[[System.Object, System.Private.CoreLib]]
-> 0000017002BBDD50 System.Object[]
-> 00000170022EF580 FunctionScreens.ProductForm

I have no idea how that is happening/possible. I brought it up here, but didn't receive any helpful responses.

Concerns were raised internally that:

A. I shouldn't let the form be closed until all operations are complete or cancelled, even with the exception handling.
B. Disposing scope in the form closing event will dispose all the object in the Form, while the Form is using them and might lead to a bad state.

Are these valid concerns? Do you see anything else wrong?

[RussKie]

I don't think dependecy injection as we know it today existed back when Windows Forms SDK was conceived (mid 90s), and getting them work together is a challenge (you've already discovered this part).

When the form is closed, the scope is closed, which in turn, starts disposing all the objects injected into the form, even if the form is still using them.

In a general case this sounds dangerous. It's quite common for forms to share common objects (such as ImageLists). For example, see #6881.
In your specific case, you'll need to analyzer and assess whether forms in your app have any shared dependecies, and consider how a disposal of a form may affect the state of the app. I think, this answers the question B.

I shouldn't let the form be closed until all operations are complete or cancelled, even with the exception handling.

It depends (c). Some situation dictate that a form can't be closed until all async operations are completed, and in other times it could be perfectly acceptable.
A slight OT. In spare time I help maintaining https://github.com/gitextensions/gitextensions/ project, and there we use https://github.com/Microsoft/vs-threading libraries (which originated from Visual Studio) to help us deal with async execution, thread management, and cancellations. It may worth looking into https://github.com/Microsoft/vs-threading, and you haven't already.

I analyzed many dumps and the form is always somehow referenced by an EF Core context (and the scope referenced below is not the local variable that I'm creating either).

If you're having an issue with EF Core object lifetime management, perhaps asking the question in https://github.com/dotnet/efcore may be a better option. /cc: @dotnet/efteam

[MisinformedDNA]

Thanks @RussKie

Is DI with WinForms not recommended? That would be hard to back out now...

Also, I looked at gitextensions, but I didn't see any DI. I also don't have any shared dependencies, and everything is created with transient scope. If the scope closing is the issue, then how do I close the scope in a safe way?

I created a simple repro app: https://github.com/MisinformedDNA/winforms-memoryleak

[RussKie]

Is DI with WinForms not recommended?

I didn't say that, I said it likely wasn't built in mind with DI (unlike ASP.NET Core, for example).
We provide an SDK that developers like yourself can use (and sometimes abuse 😄) in miriad of different ways. And some developers find ways to use the SDK with DI, e.g., #2861 may be of some interest to you.

Git Extensions uses MEF, which is a form of DI. Though it's used for composition; object lifetime management is handled via the standard Windows Forms mechanics.

[ajcvickers]

@MisinformedDNA It's worth keeping in mind two things about EF Core:

• It uses caching at the app-domain level by default for many internal structures. So even after a DbContext instance is disposed and has gone out of scope, memory will still be used by EF.
• EF uses dependency injection internally, including creating scopes. This is a separate service provider to the one used by the application.

From your sample, it also looks like the DbContext used by MainForm will never be disposed or go out of scope, so that would also lead to memory continuing to be used by EF.

Outside of the EF parts of this, I agree with @RussKie that using D.I. and WinForms like this is going to be challenging. Certainly from an EF perspective there is no requirement to ever use D.I. in the application--a DbContext can just be newed and disposed in the old-fashioned (but still very useable) way. That to me seems a better fit for WinForms.

[MisinformedDNA]

So even after a DbContext instance is disposed and has gone out of scope, memory will still be used by EF.

But it wouldn't grow linearly based on how many DbContext's where created and disposed, right? If so, this shouldn't be an issue.

EF uses dependency injection internally, including creating scopes. This is a separate service provider to the one used by the application.

Which baffles me even more as to how EF has a reference to a Form.

I agree with @RussKie that using D.I. and WinForms like this is going to be challenging

Is there a good way to use DI and WinForms? It seems no matter how I use DI with WinForms, there will always be memory leaks. If I don't create the scope, nothing ever gets disposed and I'm even worse off, right?

a DbContext can just be newed and disposed in the old-fashioned (but still very useable) way

In order to keep the code more portable, such as moving code to a web app or web service (I don't want to be stuck in WinForms forever), we are injecting EF into repositories and such. Should I just switch everything to use poor man's DI (i.e. new Form1(new SomeService(new SomeRepository(new SomeContext())));)?

So, what is the right way to do all this stuff? I hear that I am doing some stuff that is maybe "dangerous" or "challenging", but what is the best practice? Is there an enterprise level example app I can reference?

BTW: I also have a MS Support ticket open #2205190040006934