Skip to content

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

Merged
madsmtm merged 8 commits intorust-windowing:masterfrom
madsmtm:macos-main-thread-sync
Nov 30, 2022
Merged

macOS: Run tasks synchronously on main thread instead of asynchronously#2574
madsmtm merged 8 commits intorust-windowing:masterfrom
madsmtm:macos-main-thread-sync

Conversation

@madsmtm
Copy link
Copy Markdown
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 - appkit Affects the AppKit/macOS backend 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 - appkit Affects the AppKit/macOS backend S - maintenance Repaying technical debt

Development

Successfully merging this pull request may close these issues.

1 participant