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

macOS: Run tasks synchronously on main thread instead of asynchronously #2574

Merged
merged 8 commits into from
Nov 30, 2022

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Nov 29, 2022

Part of #2464.

Historically we've been using Queue::main().exec_async (dispatch_async_f), tracing back to b492564 which was included in #853. This works by putting the closure onto a queue, and then executing it on the main thread at a later point (in spirit similar to doing a thread::spawn and forget).

While this works, it is very brittle, since any code that may run after such a call will read stale data until the event loop has run once more. Instead, we'll use dispatch_sync_f, which will block the calling thread until the main thread has had a chance to run it, meaning that whatever state changes may have happened are visible right after any call.

I've left out the things that affect the fullscreen logic, since that is more gnarly, and would be nice to have as a separate commit to allow easier bisection if something goes wrong. Will follow up with another PR for that.

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users

Note: I strongly suspect the reason that dispatch_async_f was originally chosen over dispatch_sync_f was because dispatch_async_f works on the main thread out-of-the-box - though @francesca64 may be able to clarify on that.

@madsmtm madsmtm added B - bug Dang, that shouldn't have happened DS - macos S - maintenance Repaying technical debt labels Nov 29, 2022
@madsmtm madsmtm merged commit bf92f3e into rust-windowing:master Nov 30, 2022
@madsmtm madsmtm deleted the macos-main-thread-sync branch November 30, 2022 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened DS - macos S - maintenance Repaying technical debt
Development

Successfully merging this pull request may close these issues.

1 participant