winit icon indicating copy to clipboard operation
winit copied to clipboard

Return immediately from `run_app` on web

Open madsmtm opened this issue 9 months ago • 11 comments

Builds upon https://github.com/rust-windowing/winit/pull/4149.

Returning immediately from EventLoop::run_app on the web avoids using JavaScript exceptions, which is a huge hack that has compatibility issues, and doesn't work with the Exception Handling Proposal for WebAssembly.

This needs the application handler passed to run_app to be 'static, but that works better on iOS too anyhow (since you can't accidentally forget to pass in state that then wouldn't be dropped when terminating). This effectively reverts the decision in https://github.com/rust-windowing/winit/pull/3006, CC @kchibisov, do you recall if there was a deep motivation for doing that?

Since spawn_app (added in https://github.com/rust-windowing/winit/pull/2208) is now no longer necessary, I've removed it. This means that all the examples should work properly on web again.

  • [x] Tested on all platforms changed
  • [x] Added an entry to the changelog module if knowledge of this change could be valuable to users
  • [x] Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • [x] Created or updated an example program if it would help users understand this functionality

madsmtm avatar Mar 17 '25 04:03 madsmtm

I think they worked properly as well? Just we had a hack to hold the exit? In general, the point was that you might have a code that does cleanup after the exit in a cross platform manner, and thus, doing an instant return doesn't make much sense from a cross platform behavior.

The iOS model is acceptable, since you close the app anyway after so. In general, I'm leaning towards special backends just not being a part of the regular run facility at all, so the difference is clearly stated for them.

kchibisov avatar Mar 17 '25 09:03 kchibisov

I think they worked properly as well? Just we had a hack to hold the exit?

The hack was "throw an exception and cross fingers that Rust doesn't see it and run destructors". Not really what I'd call "worked properly", rather "terribly UB but worked because WASM doesn't yet support exceptions".

In general, the point was that you might have a code that does cleanup after the exit in a cross platform manner, and thus, doing an instant return doesn't make much sense from a cross platform behavior.

On the web and iOS currently, having cleanup after run_app is wrong. If using spawn_app, having cleanup after is even more wrong. If we want to provide a cross-platform API here, then the user just shouldn't do anything after run_app.

The iOS model is acceptable, since you close the app anyway after so. In general, I'm leaning towards special backends just not being a part of the regular run facility at all, so the difference is clearly stated for them.

My problem is that example code suffers when we do not provide a single API that everyone can reliably use (Android is special in that it cannot be a binary, but the others do not need to suffer from this). An alternative would be to provide EventLoop::run_or_spawn_app, but that seemed unnecessary to me.

madsmtm avatar Mar 17 '25 09:03 madsmtm

On the web and iOS currently, having cleanup after run_app is wrong. If using spawn_app, having cleanup after is even more wrong. If we want to provide a cross-platform API here, then the user just shouldn't do anything after run_app.

Yeah, but those targets are rather special, so I don't see an issue with them being treated via their own API. Web needs special code anyway to setup canvas and such. We may change it to the way you suggest, but I'm just worrying that it could be more surprising, since I'm already not much in favor that semantics do differ.

kchibisov avatar Mar 17 '25 10:03 kchibisov

An alternative would be to provide a winit_main!(|| App::default()); macro that initializes the application and runs the event loop under default settings. This would probably also be a bit nicer on Android.


As a few data points, Bevy is already effectively merging run_app and spawn_app, so is Iced. Masonry/Xilem isn't using spawn_app, though they do support web (so they get the worse behaviour), same with ggez. eframe uses just run_app, but their web impl bypasses Winit anyhow. softbuffer's examples have a dedicated utility function just for this.

My point is that all of these examples would benefit from this PR.

madsmtm avatar Mar 17 '25 10:03 madsmtm

I'm just worrying that it could be more surprising, since I'm already not much in favor that semantics do differ.

I do somewhat agree here, the differing semantics are confusing (I've tried to remediate this with better documentation), though I guess my point is that to most users it probably won't matter (either they only support desktop, and then they don't care, or they also support web, in which case they'll have tested their app there and seen that it does indeed work as they expect).

madsmtm avatar Mar 17 '25 10:03 madsmtm

I don't like this solution at all, but I think its the best we can do right now

Wondering, in the ideal world, how would you like Winit's cross-platform entry point to be?

madsmtm avatar Mar 28 '25 00:03 madsmtm

I don't like this solution at all, but I think its the best we can do right now

Wondering, in the ideal world, how would you like Winit's cross-platform entry point to be?

So in the future, with the stack-switching proposal, Web could actually block and we could have a returning function like with every other backend except iOS.

Because of iOS not able to return after the run_app() function, I believe the only truly cross-platform API we ever had was the non-returning one, which we removed in https://github.com/rust-windowing/winit/pull/2767 because of issues around Android (https://github.com/rust-windowing/winit/issues/2709). So my preference is actually to go back to that. We have briefly discussed this in today's meeting but couldn't come to a conclusion without an Android expert.

If we can't have a cross-platform API that behaves the same on every platform, my next preference is to just not have a cross-platform API. One argument against that is that users often just ignore the differences like this:

#[cfg(not(target_family = "wasm"))]
event_loop.run_app(App::default())?;
#[cfg(target_family = "wasm")]
event_loop.spawn_app(App::default());

So we agreed that if we can't find a truly cross-platform API, we will go ahead with this PR's proposal.

daxpedda avatar Mar 28 '25 07:03 daxpedda

To expand on iOS: The only public API that Apple provides is UIApplicationMain.

I can think of several workarounds to return to the user's code anyhow upon termination, but all of them have costs that I don't think Winit should pay.

  1. Use private APIs. This may get the user's app rejected from being published to the App Store, and while that can probably be worked around with dlsym, it is brittle anyhow since, well, the APIs are private, so Apple might change them in the future.
  2. Panic inside on termination, and catch that panic outside. Haven't tested, might work depending on internals in how UIApplicationMain deals with non-Objective-C, non-C++ exceptions. Also dependent on Rust's unwind handling details.
    • Won't play nicely if UIKit expects to run code after invoking the user's termination handlers (e.g. the application could easily be marked by the OS as "crashed", and end up sending a report to the developer. I'm unsure here though).
  3. setjmp before UIApplicationMain, longjmp on termination. Same issues as above with code after termination, also won't run Apple's destructors.
  4. Maybe longjmp inside an exit signal handler? Seems very UB-prone.

madsmtm avatar Mar 28 '25 08:03 madsmtm

We discussed this a few months ago, the resolution back then was:

@daxpedda will talk to @MarijnS95 about Android and process::exit, otherwise we'll go @madsmtm's PR.

Has this happened? If not, I'd like to move forwards with this PR.

So in the future, with the stack-switching proposal, Web could actually block and we could have a returning function like with every other backend except iOS.

I think that'd be fine with this PR too? We'd document that without #[cfg(feature = "stack-switching")], run_app returns immediately on Web, and with it, it blocks until it's finished just like the other platforms.

madsmtm avatar Jun 07 '25 20:06 madsmtm

@daxpedda will talk to @MarijnS95 about Android and process::exit, otherwise we'll go @madsmtm's PR.

Has this happened? If not, I'd like to move forwards with this PR.

I wasn't unable to catch him yet.

So in the future, with the stack-switching proposal, Web could actually block and we could have a returning function like with every other backend except iOS.

I think that'd be fine with this PR too? We'd document that without #[cfg(feature = "stack-switching")], run_app returns immediately on Web, and with it, it blocks until it's finished just like the other platforms.

Turns out my hopes for stack-switching were too high. I do not believe this will be realistically able to resolve this. Using this proposal will require the function to become async on the JS side. Which is e.g. fine for fn main(), but because its not really possible for a compiler to somehow mark all functions that are using stack-switching with async, we end up with simply all functions being exported with async.

TLDR: AFAICS stack-switching will not solve this.


I'm strongly in favor of going back to the old model. It seems to me the only one that is consistent between all platforms and correct. My understanding is that while it is not ideal to process::exit() on Android, because it might shut down other activities or the like, it isn't incorrect. Instead we should expose a platform-specific function on Android, like spawn() for Web, for exactly that use case.

Maybe there could even be a way to do this activity detection at run-time and block until other activities have shut-down, but I really have no idea what I'm talking about. I will ping Marijn again.

WDYT?

daxpedda avatar Jun 08 '25 08:06 daxpedda

I forgot to mention that there is a way to get rid of the exception-handling guard. We can add unwind-safety the same way Std does: by just adding a struct that on Drop calls unreachable!(). So when a user tries to catch the thrown exception via unwinding, it will trigger our run-time guard.

I think it should also be possible to somehow prevent auto implementation of UnwindSafe and move this check to compile-time, but I don't know exactly how this is done for function/method calls.

daxpedda avatar Jun 08 '25 18:06 daxpedda

~~I thought we discussed this in the last meeting and concluded that we won't do this? Or what are you planning to do in this PR?~~

EDIT: apologies! Just saw that you commented, let me read first.

daxpedda avatar Aug 30 '25 16:08 daxpedda

My understanding is that while it is not ideal to process::exit() on Android, because it might shut down other activities or the like, it isn't incorrect.

Short circuiting the Activity lifecycle has a user-visible effect on application switching and bypasses the ability for an Androd application save state on shutdown. It really is incorrect to allow run() to exit the process on Android. Winit can't assume responsibility for the whole process. An android-activity app is given a main thread that is conceptually subordinate to the Java thread and needs to be able to gracefully return from android_main, or worst case needs to panic the main thread so that can be caught by android-activity which can call Activity::finish() on the Java thread. Alternatively, Android should not support a run API that can't return.

On the other hand, it should always be valid that a Rust API returning a Result can behave like a function that doesn't return, which is why I've previously argued that the most portable choice is to have a run api that returns a Result (but on iOS and Web would act like a -> ! API). That should be the most-portable, least bad option, even though developers need to be aware that on iOS and Web they can't expect code after the run API to ever be reached.

In the end there's no option that's portable across all platforms. -> ! isn't portable to Android and -> Result might give the impression you will be able to observe the Result on all platforms (not possible on iOS or Web). But it's invalid (not a compromise) to exit() the process on Android, so the least bad choice seems to be an api that "returns" a Result (just not in a finite amount of time on some platforms).

rib avatar Aug 31 '25 16:08 rib

With incorrect I meant UB here.

An android-activity app is given a main thread that is conceptually subordinate to the Java thread and needs to be able to gracefully return from android_main, or worst case needs to panic the main thread so that can be caught by android-activity which can call Activity::finish() on the Java thread.

Can this be actually caught? I'm also not sure what android activity does report back to the system, because AFAIU if its reported that the process failed the app might just restart.

Alternatively, Android should not support a run API that can't return.

If we deem these drawbacks too steep, this is a good alternative.

On the other hand, it should always be valid that a Rust API returning a Result can behave like a function that doesn't return, which is why I've previously argued that the most portable choice is to have a run api that returns a Result (but on iOS and Web would act like a -> ! API). That should be the most-portable, least bad option, even though developers need to be aware that on iOS and Web they can't expect code after the run API to ever be reached.

While its true that this isn't wrong as per the Rust rules, I think its terribly misleading. In my opinion, if we can't make Android work with std::process::exit(), we should go with the alternative of just not providing this cross-platform API on Android.


You mentioned that process::exit() would prevent Android save state on shutdown. While I think this is a terrible trade-off, it doesn't seem "wrong" to me. So if users want to use the Android save state, they just have to use the dedicated API.

You also talked about the whole thread ownership issue, but I don't know exactly what kind of side-effects this actually has. Again: my understanding is that its not "wrong" in the sense of you can work with it and its not UB.

Obviously UB is not the line we should draw here, it should be possible to produce a correctly working app with this setup. Even if some features won't work.

I also heard that there are ways to solve this on the backend side, like waiting for the activity to finish in Winit. But this would require work on android-activity and such which we don't have anybody handling right now.

I really want to emphasize that I really don't know what I'm talking about here and the information I gathered so far is close to hearsay, so I really appreciate every drop of input.


Just to count off all options right now:

  1. One cross-platform method that has different return behavior depending on the platform.
  2. One cross-platform method that does process::exit() on all platforms. With Android being in a bad position.
  3. Multi-platform methods with different return behavior.
    fn main() -> Result<()> {
        ...
    
        if cfg!(target_family = "wasm") {
            event_loop.run_and_return_immediately()
        } else if cfg!(target_os = "ios") {
            event_loop.run_and_never_return()
        } else {
            event_loop.run_and_return_once_done()
        }
    }
    
  4. A proc-macro over fn main() like so:
    #[winit::main]
    fn main() -> impl Application {
        App {}
    }
    

So 1. is what we currently have and want to move away from. 2. is what we are currently discussing on going with. 3. and 4. is something we are discussing going for regardless.

So if 2. isn't going to work, we will still have 3. and 4. which will work for most users. E.g. this will work on all platforms except Android:

fn main() {
    ...

    event_loop.run_and_never_return();
}

daxpedda avatar Aug 31 '25 19:08 daxpedda