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

[FEATURE REQUEST] Allow panics in places like MainContext::spawn_local to propogate up to the caller #1229

Closed
hwittenborn opened this issue Nov 22, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@hwittenborn
Copy link

hwittenborn commented Nov 22, 2023

Currently MainContext::spawn_local won't propagate panics up to the caller if those future's aren't awaited and .unwrap()ed. I have a use case where I'd like to get panics in those locations up to the root of my application though (see Relm4/Relm4#578), and I haven't found any current way to implement that with what glib provides.

I've looked around loosely at the places catch_unwind is used throughout this codebase, but I was mostly left clueless on how I might change the functionality to propagate panics up. How hard would this functionality be to add?

@hwittenborn hwittenborn added the enhancement New feature or request label Nov 22, 2023
@sdroege
Copy link
Member

sdroege commented Nov 22, 2023

You want those panics to propagate up to e.g. main_loop.run(), or where exactly?

@hwittenborn
Copy link
Author

hwittenborn commented Nov 22, 2023

I'm pretty sure just from MainContext::iteration to the caller, though I'm not sure if/how MainLoop::run gets involved with that. I'm just wanting panics to not get caught at all, though I'm not sure what that'd look like in glib's codebase.

Specifically I'm wanting this code to crash completely in the first execution of the loop, instead of managing to go through all five runs:

use glib::MainContext;

fn main() {
    let ctx = MainContext::new();
    ctx.spawn(async {
        panic!("OH NO!");
    });
    
    for i in 0..5 {
        println!("DID WORK #{i}: {}", ctx.iteration(false));
    }
}

@sdroege
Copy link
Member

sdroege commented Nov 22, 2023

We can't really have the panics propagate up like this as the GMainLoop (C) code is not unwind-safe. However it should be possible to directly abort the process on panic, one way or another.

The problem here is the JoinHandle collecting the panic but you're not actually doing anything with the join handle. The behaviour we have now is consistent with how tokio and async-std handle the same situation.

@hwittenborn
Copy link
Author

hwittenborn commented Nov 22, 2023

We can't really have the panics propagate up like this as the GMainLoop (C) code is not unwind-safe. However it should be possible to directly abort the process on panic, one way or another.

I'd be fine with aborting as well, the way things are being handled in Relm4/Relm4#578 is just causing my application to not die out (as the panics aren't processed from the JoinHandle like you said). I'm pretty sure Relm4 can't wait on the future to resolve as it would cause some issues with certain things blocking, but I'm not entirely positive.

The behaviour we have now is consistent with how tokio and async-std handle the same situation.

I understand that, but it's still something I think could be really useful in certain situations (like mine). I can't really think of another way of doing this all in a clean manner, and having this kind of functionality in glib would definitely help to keep my codebase simplified.

@AaronErhardt
Copy link
Contributor

I'm pretty sure Relm4 can't wait on the future to resolve as it would cause some issues with certain things blocking, but I'm not entirely positive.

Actually that should be possible. We could spawn an async task that receives all join handles and waits for them to complete. When a panic happens, we could execute some code from there.

IMO there is nothing that needs to be changed in gtk-rs. As it was pointed out, basically all async runtimes catch unwinds and with the C-code in between, it is not feasible to change that anyway. Maybe adding an option for panic=abort would be useful though.

@hwittenborn
Copy link
Author

We can't really have the panics propagate up like this as the GMainLoop (C) code is not unwind-safe. However it should be possible to directly abort the process on panic, one way or another.

Looking at this again, since the only option is to abort this is pretty easily solvable in my program by just adding panic = "abort" to my Cargo.toml file. It looks like this is pretty much solved on my end - if there's something else you think should be changed though @sdroege, feel free to reopen the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants