Return an error when Windows event loop is busy

[amrbashir]

This was a regression introduced in #3418

This new error could be used to retry events at later time

This also removes EventLoopClosed struct
and replaces it with EventLoopProxyError enum

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

[madsmtm]

Hmm, I'm unsure how you are expected to handle this as a user? The event is still in the queue, we can't just try to handle it later?

Also, this somewhat conflicts with #3424, I would really like the wakeup mechanism that we use to not be able to fail.

[amrbashir]

Hmm, I'm unsure how you are expected to handle this as a user? The event is still in the queue, we can't just try to handle it later?

I initially started this PR with an older commit without noticing, it used to mess up the order of events, but no I am having a hard time reproducing it, will convert this to a draft until I can reproduce. (sorry for then noise)

Also, this somewhat conflicts with #3424, I would really like the wakeup mechanism that we use to not be able to fail.

yeah that would be great but PostMessageW has 10,000 limit so it will always fail with a lot of events so users will need to retry events (or at least a dummy event) or wait for the OS to send an event so the events in queue could be dispatched

[amrbashir]

I have added an example showcasing the problem, basically even though the events are not lost and are sitting in the queue, we need to send empty wakeup events so they can be dispatched again. Without this PR, we can't know if PostMessageW fails and so can't send empty wakeup events.

The example tries to send 15000 events, it fails to deliever 2667 of them and we need to send at least another successful 2667 equivalent wakup events

[madsmtm]

We're reworking user events in #3694, in there I noted an alternative:

Maybe on Windows, we can keep a wake_up_failed: Arc<AtomicBool> around, and then do it in the next iteration if the boolean is set?

Do you think that's a viable solution to this problem?

[amrbashir]

Quickly reading through the changes in #3694, it still exhibits the same problem, but it is a lot better since it won't consume the data being sent.

Maybe on Windows, we can keep a wake_up_failed: Arc<AtomicBool> around, and then do it in the next iteration if the boolean is set?

Do you think that's a viable solution to this problem?

Not sure what you mean by "do it in the next iteration", do you mean call ApplicationHandler::user_wakeup_event method one more time?

I don't think that helps much, since more than one wakeup events can fail consecutively.

I could probably change my logic to use for event in rx.iter() to empty out the pending events once a successful wakeup has been through but still I would need to know that wakup failed by returning an error if PostMessageW failed, so I can run a timeout and wake the event loop again later.

[madsmtm]

Not sure what you mean by "do it in the next iteration", do you mean call ApplicationHandler::user_wakeup_event method one more time?

I meant to, if PostMessageW failed, set a variable somewhere that is then checked at the top-level inside public_window_callback (or thread_event_target_callback, not sure what the difference is) and if it is set, then emit the event.

This would work under the assumption that the reason why PostMessageW fails is that it's busy processing other events, and as such we don't actually need to wake up the event loop, it is already wide awake.

[amrbashir]

Not sure what you mean by "do it in the next iteration", do you mean call ApplicationHandler::user_wakeup_event method one more time?

I meant to, if PostMessageW failed, set a variable somewhere that is then checked at the top-level inside public_window_callback (or thread_event_target_callback, not sure what the difference is) and if it is set, then emit the event.

But why not just return an error if it failed, ofc I am talking in the context of #3694 where the proxy just calls PostMessageW so you can check if it failed and return an error

[madsmtm]

But why not just return an error if it failed, ofc I am talking in the context of #3694 where the proxy just calls PostMessageW so you can check if it failed and return an error

Hmm, mostly because it's unclear how the user is expected to handle that error? Like, they can't just retry, waking up, as that'll likely just give them the same error.

Especially so if there is a solution for us to handle this fully internally in Winit, then I'd rather not push the error case onto users.

[amrbashir]

But why not just return an error if it failed, ofc I am talking in the context of #3694 where the proxy just calls PostMessageW so you can check if it failed and return an error

Hmm, mostly because it's unclear how the user is expected to handle that error? Like, they can't just retry, waking up, as that'll likely just give them the same error.

Yeah that is expected, and in that case we have a logic to try which re-tries until succession

let mut res = wakeup_eventloop();
while (res.is_err()) {
  sleep(15ms);
  res = wakeup_eventloop();
}

This is a very basic solution and maybe sufficient along with a receiver implementation that uses for event in rx.iter(), but if needed we probably will add a state to track how many calls failed and we will dispatch equivalent number of wakeup calls.

Especially so if there is a solution for us to handle this fully internally in Winit, then I'd rather not push the error case onto users.

I don't see how Winit could handle this case tbh, especially since winit will no longer take ownership of the data, and if wakeup failed, user can't know if wakeup failed or which one failed and can't implement logic to gracefully handle that.

[madsmtm]

we will dispatch equivalent number of wakeup calls.

We haven't finalized this yet, but I think we're going to decide that multiple wakeup calls don't necessarily lead to multiple wakeups; i.e. they may be merged.

I don't see how Winit could handle this case tbh

I'll try to whip up another PR for it once #3694 is done (especially if reminded), so let's discuss it at that time.

[amrbashir]

We haven't finalized this yet, but I think we're going to decide that multiple wakeup calls don't necessarily lead to multiple wakeups; i.e. they may be merged.

That should still be fine for our use-case.

I'll try to whip up another PR for it once #3694 is done (especially if reminded), so let's discuss it at that time.

Thanks, I will keep an eye for it

[daxpedda]

Especially so if there is a solution for us to handle this fully internally in Winit, then I'd rather not push the error case onto users.

I very much agree with this.

[amrbashir]

closing since #3694 is merged

[amrbashir]

@madsmtm although #3694 is merged, EventLoopProxy::wake_up can still fail and this failure is not propagated to caller, and so we can't know if we should retry waking up or not. Should I open a PR or this still not wanted?

[kchibisov]

In general, the issue is that error won't help in case you call wake_up from the event loop thread, since otherwise you'll simply busy-loop and the error handling is meaningless in your case.

The goal was to deal with errors and re-tries internally inside the winit, since it's much more clear for the users and winit certainly can figure that out.

In windows case, I'd generally see how the handling could be improved. Likely, one can share Arc or something to indicate that, since the loop is likely not sleeping during the error to post the message, so it can check it.

There could be other means to communicate that on windows and deal with errors internally.

[amrbashir]

In general, the issue is that error won't help in case you call wake_up from the event loop thread, since otherwise you'll simply busy-loop and the error handling is meaningless in your case.

In tauri, we have a check to see if we are on the event loop thread, and decide whether to execute our logic or send it to be executed on the main thread. So we are never waking the event loop from within the event loop and we can retry if failed.

I am fine with winit handling this internally if possible.

[madsmtm]

I've opened #4150 to fix this properly.