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

[API Proposal]: Dispose Application in Generated Entry Point #10074

Open
koenigst opened this issue Nov 15, 2024 · 7 comments
Open

[API Proposal]: Dispose Application in Generated Entry Point #10074

koenigst opened this issue Nov 15, 2024 · 7 comments
Labels
API suggestion Early API idea and discussion, it is NOT ready for implementation

Comments

@koenigst
Copy link

Background and motivation

If any disposable fields are present in the application (i.e. App.xaml.cs) the default analyzers will suggests implementing IDisposable on the application class. The code generator for the entry point creates the application without disposing it at the end. This might confuse developers.

The fix would be quite simple by just generating a using statement if the application class implements IDisposable.

A workaround is possible by creating a custom entry point.

API Proposal

Generated output:

/// <summary>
/// Application Entry Point.
/// </summary>
[System.STAThreadAttribute()]
[System.Diagnostics.DebuggerNonUserCodeAttribute()]
[System.CodeDom.Compiler.GeneratedCodeAttribute("PresentationBuildTasks", "8.0.5.0")]
public static void Main() {
	SplashScreen splashScreen = new SplashScreen("splash.png");
	splashScreen.Show(true);
	using (App app = new App())
	{
		app.InitializeComponent();
		app.Run();
	}
}

API Usage

public partial class App : Application, IDisposable
{
    public void Dispose()
    {
        // Dispose resources
    }
}

Alternative Designs

No response

Risks

Application classes already implementing IDisposable will get an additional call to Dispose. This should be a small issue as Dispose should be safe to call multiple times.

@lindexi
Copy link
Member

lindexi commented Nov 16, 2024

@koenigst I do not think it should dispose. Because we dispose the application before we exit.

@koenigst
Copy link
Author

@koenigst I do not think it should dispose. Because we dispose the application before we exit.

@lindexi The application is disposed where? My experience is the opposite.

@lindexi
Copy link
Member

lindexi commented Nov 16, 2024

@koenigst What I mean is that the process has exited and the resource does not need to be released.

@koenigst
Copy link
Author

@lindexi true, that is sufficient for resources cleaned up the os. I had issues with Dispose implementations that did more work such as flushing a buffer (e.g. NLog's logging buffer).

@h3xds1nz
Copy link
Contributor

You've got multiple events raised to do your cleanups (e.g. flushing buffers) in and just making it IDisposable imho doesn't 100% solve the issue and just adds additional noise.

Pick your poison, based on the expected lifetime:

Dispatcher.ShutdownStarted (if you're using multiple dispatchers, just main the main one)
Application.Exit (sibling to Application.Startup)
AppDomain.ProcessExit

Your original question doesn't raise the concern, but it's good to consult Application.SessionEnding as well for OS exits / user log offs, it should wait for processing iirc unless it is a forced shutdown.

@koenigst
Copy link
Author

These events do allow for various ways to do cleanup. Another possibility would be just to write a custom entry point where the application is disposed.

I do think that having a simple solution out of the box would be advantageous for developers who do not have the same depth of understanding. The analyzers will suggest implementing IDisposable on the application and that is a pattern people coming from ASP.NET are used to.

I also think it is a low effort and low risk change.

koenigst added a commit to koenigst/wpf that referenced this issue Nov 19, 2024
@h3xds1nz
Copy link
Contributor

Application is a singleton; for myself, I don't find it a good practice to implement IDisposable pattern on them in the first place.

Plus, Dispose won't give you the level of control and information that a windows application developer should have in mind.

  • e.g. do different kind of operations on log off/shutdown (that can include session0 delegation, etc.).

Your argument for abstracting those things away I understand, especially if you're coming from ASP.NET, after all, that's kinda Framework's job; but polluting the API surface with implementing IDisposable is not the way in this very case imho. Either you don't care how your app starts/exits or you shall be explicit about both and understand the different approaches.

Chances are if you need to clean up something on app exit, you need to initialize those things at startup as well, otherwise the lifetime is not very well defined and that is another issue.

@harshit7962 harshit7962 added the API suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API suggestion Early API idea and discussion, it is NOT ready for implementation
Projects
None yet
Development

No branches or pull requests

4 participants