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

Signal handling … again?! #40

Closed piegamesde closed 3 years ago

piegamesde commented 3 years ago

The current solution works fine in all of my use cases, albeit it occasionally requires rather ugly code. But since the last signal handling overhaul there is a tiny voice in my head keeping me awake at night, and it says "it could be even better". It also says that the implementation will probably require a bit of unsafe code …

The problem is with woab::spawn_outside and the fact that that it queues events for to be processed later on. While this is a common approach to Actix message passing, it is not what I am used from GTK, where all events are processed right away. The switch from one to the other only caused a handful of bugs, but still.

The trick is the following: if we take a &mut self to make sure that the Actor is not borrowed otherwise anymore, we can safely run a nested callback in the Actix engine. This will still require a call wrapper like woab::spawn_outside and may look like this:

woab::run_gtk(
    &mut self,
    clone!(@weak carousel => @default-panic, move || {
        for page in &carousel.get_children()[new_pages..old_pages] {
            carousel.remove(page);
        }
    }),
);

Alternative syntax:

self.run_gtk(
    clone!(@weak carousel => @default-panic, move || {
        for page in &carousel.get_children()[new_pages..old_pages] {
            carousel.remove(page);
        }
    }),
);

let carousel = self.widgets.carousel.clone();
woab::run_gtk!{
    for page in &carousel.get_children()[new_pages..old_pages] {
        carousel.remove(page);
    }
};

I can think of a few other ways to make calling this more pleasant but that's not the point right now. The point is that after this method invocation, the called code continues, and it knows that this closure has been executed. This is the crucial difference to spawn_outside, where every code needs to be aware of the fact that the changes have not been "committed" yet.

idanarye commented 3 years ago

Sadly, that won't work. The main reason for woab::spawn_outside is wanting to do stuff that would cause GTK to emit signals - which we won't be able to handle while Actix is running. woab::run_gtk may be able to free the Actix runtime to handle the signal - but if GTK needs to route a signal to the actor we are blocking - it'd deadlock. And seeing how it's common sense that the same actor that's in charge of some widgets is also the one receiving their signals - this is not just a possibility of deadlock, it's closer to certainty!

Have you checked out woab::outside? Unlike woab::spawn_outside, woab::outside is a Future so if you run it in a Future that runs on the Actix runtime, you can .await it and do stuff after it - including going back into the actor.

idanarye commented 3 years ago

I have a better idea - see #41.

piegamesde commented 3 years ago

So who is blocking the actor from handling a nested GTK signal? I thought it was the Rust ownership system—but that can be worked around using unsafe, and the abstraction can be made safe by borrowing the actor &mut self in woab::run_gtk.

idanarye commented 3 years ago

That would depend on how Actix implements its actor mechanism internally. Which I don't want to depend on - especially when invoking unsafe code.

piegamesde commented 3 years ago

Okay, I see. There are ways to get around this which are acceptable when the nested signal targets the own actor, but for the others I don't see a way that does not hack on the internals of Actix. Closing in favor of #41.