idanarye / woab

Widgets on Actors Bridge - a GUI microframework for combining GTK with Actix
https://crates.io/crates/woab
MIT License
14 stars 1 forks source link

Shutdown handling #45

Closed piegamesde closed 3 years ago

piegamesde commented 3 years ago

I'm working on panic handling in my application, and this especially involves properly shutting down Gtk. During this, I noticed that currently woab only shuts down (= stop all the actors) when the application exits. This is no longer appropriate for my application, because I want to run code between Gtk shutdown and application exit.

idanarye commented 3 years ago

What exactly do you want here? Once GTK shuts down, it will no longer send signals and no longer trigger the idle handler that cranks the Actix runtime. So the actors are still "up" but they are not going to get any more execution time.

Or do you need a way to invoke System.stop on the Actix runtime WoAB is using?

piegamesde commented 3 years ago

Or do you need a way to invoke System.stop on the Actix runtime WoAB is using?

Effectively, yes. Users have to run run_actix_inside_gtk_event_loop as a pre-GTK hook, so a post-GTK hook that does the appropriate shutdown would be good to have. Maybe we can have a shutdown hook on application, but not sure how reliable this would be.

idanarye commented 3 years ago

Can do. I'm not going to automatically hook it up to application shutdown - that should be easy enough to do once I provide the function, and I see no benefit in WoAB doing that registration for you.

piegamesde commented 3 years ago

Okay, I found an issue that I didn't think of previously: There is no way to query whether the woab engine is running. An attempt to shut down a non-running engine will result in a panic.

This means that when I write this code in the panic hook, and my application panics before WoAB got started, I end up in a double panic abort.

idanarye commented 3 years ago

woab::close_actix_runtime currently returns a Result<(), std::io::Error>, where the error is what actix::SystemRunner::run would return (I don't know in which cases - I guess it depends on stuff that happen inside the actors?)

I think it is justified to have nested Result here:

pub fn close_actix_runtime() -> Result<Result<(), std::io::Error>, StopError> {

Where:

#[derive(thiserror::Error, Debug)]
pub enum StopError {
    #[error("Cannot stop the WoAB runtime because it was not started")]
    RuntimeNotStarted,
    #[error("Cannot stop the WoAB runtime because it is currently in use. Try stopping it with `actix::System::current().stop();` instead")]
    RuntimeInUse,
}

Then you can match on the outer Result and decide to either not stop the WoAB runtime or stop it with actix::System::current().stop().

The reason I don't want to actix::System::current().stop(); inside woab::close_actix_runtime when the runtime is being in use is that I won't be able to wait for it to finish. This will have to be done externally somehow. I'm not sure you even can - if you are doing it in a panic handler, then maybe the runtime got into an illegal state and cannot finish?

piegamesde commented 3 years ago

I thought about doing a nested Result, but using an Other(io::Error) variant would be fine for me too. Regardless, I think we should still provide a method to query WoAB's running state because somebody is going to need it sooner or later for some reason or another.

I'm not sure you even can - if you are doing it in a panic handler, then maybe the runtime got into an illegal state and cannot finish?

Apart from the fact that panics may happen on other threads (btw I should check what happens in that case due to Gtk thread safety), it has worked surprisingly well so far.