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

Move IDispatcher logic out of IPlatformServices #2862

Merged
merged 56 commits into from
Nov 16, 2021
Merged

Conversation

mattleibow
Copy link
Member

@mattleibow mattleibow commented Oct 7, 2021

Description of Change

This PR moves the dispatcher and "run on main thread" logic out of the Forms/Device/IPlatformServices space and into Core and Controls.

Core has the basic logic for executing on a "platform amin thread" and Controls attaches that logic to all BindableObjects.

Part of #1965

Discussion

Due to backwards compatibility, I have actually implemented this "wrong" in the objective sense. This is because we allow UI objects to be created and used off the UI thread.

For example, If I start up a background thread and on that background thread I create a Button. Firstly, that is incorrect because logic in the constructor is now running on a background thread. Second, when I use the Dispatcher property, what thread should I use? Right now I just thrash around and try things until I find one.

Now, the thing that sort of holds this together is that there typically is only 1 UI thread. In most cases. Even in Windows with multiple windows because the default way we create windows is on the same thread. So we are lucky. However, this is not fixed. The user or maui may opt to create windows in separate threads - especially if the windows do a lot of work.

We then may enter a world of pain. For example this execution:

  1. Assume on UI thread 1
  2. Create background thread B
  3. Create a button myButton on B
  4. App creates a second window on a new UI thread 2
  5. App executes myButton.Dispatcher.Invoke from B (probably will pick UI 1)
  6. App closes first window on thread 1
  7. App executes myButton.Dispatcher.Invoke from B :
    1. App picks UI 1 because it is cached and will throw
    2. App picks UI 2 because we decide not to cache
  8. We now have one UI element in a background thread that either crashed or executed on another thread and have lost consistency

Additions made

  • Moved IDispatcher and IDispatcherProvider to Core.
  • Added specific instances of dispatcher objects to the IMauiContext.
  • Moved the very weirdly-specific logic out of the MauiContext ctors and into the correct scoping methods as they fit better there.
  • Obsoleted the various methods on Device.
  • Update the usage of Device.* to use the dispatchers.

@PureWeen PureWeen requested a review from hartez October 7, 2021 18:14
@rmarinho rmarinho added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Oct 8, 2021
@mattleibow mattleibow changed the title Move IDispatcher logic out of IPlatformServices Move IDispatcher logic out of IPlatformServices Oct 15, 2021
@KPixel
Copy link

KPixel commented Nov 11, 2021

I wasn't aware that this was possible in Xamarin.Forms. If it isn't desired in MAUI, maybe we can keep it only in compatibility mode (opt-in). Or, if it is still necessary, I just hope that it doesn't "compromise" the normal way of building UI on their UI thread.

I am waiting for multi-window and CollectionView to be available on Windows (in Preview 11 hopefully). Then, I will port my app to MAUI and see if any of these potential issues still occur.

Edit: I'm glad to see Device.BeginInvokeOnMainThread() go away; it was the cause of many bugs. Application.Current.Dispatcher should also be avoided as much as possible for the same reason.

@PureWeen PureWeen merged commit c1525fa into main Nov 16, 2021
@PureWeen PureWeen deleted the dev/dispatchers branch November 16, 2021 16:04
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
@samhouts samhouts added the fixed-in-6.0.101-preview.11.3 Look for this fix in 6.0.101-preview.11.3! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-6.0.101-preview.11.3 Look for this fix in 6.0.101-preview.11.3! legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants