Open dotMorten opened 5 years ago
This is a very valid concern, thank you for raising it up. Thread affinity support for DPs in the framework may not be a big problem, but may very well be for implementers of INPC.
If some code is able to raise a property change from a non-UI thread, it also means that the backing code of the property may be changed from a background thread. It may be protected it using locks, but two-way bindings can then become a problem, as they may make property change chains deadlock.
There's also the biggest problem of INPC, being the lack of "current" value when the event was raised. There can be situations where properties are not synchronized properly, (e.g. SelectedIndex
and ItemsSource
not updated synchronously and SelectedIndex
loses its value), or with ping-pongs of changes caused by property states (e.g. TextBox changes and regex filters).
This then brings the issues with INCC, where the whole collection needs to be protected properly. For instance, you may raise an event referencing an item that has been moved, or is not there anymore.
I'm not saying there are no solution to this problem (there definitely are) but IPNC listeners threading affinity changes are not enough to make this problem easy on both ends of the interface.
WPF has this figured out so perhaps just pull the solution from there?
Great suggestion. We do this for CollectionChanged events (somewhat by accident...), we should be able to do the same for NotifyPropertyChanged.
Agreed we should use the WPF solution (plus the same thing for x:Bind).
INPC is pretty straightforward. We have a partial solution for INCC today as Jevan said by accident (the change handler is an apartment object so it marshals to the UI thread), but it's not quite correct; the args tell of the changed item by index, and by the time the it gets to the UI thread the index might have changed. This is solved in WPF we should do the same thing.
Similar issue #2795
Bumping, though think this is a feature request, based on https://github.com/microsoft/microsoft-ui-xaml/discussions/8638
Summary
Allow UI bindings to happen from properties that raises notification changes from any non-UI Thread
Rationale
When you're binding properties, you must always raise property change notification on the UI Thread, or you'll get a runtime exception. This causes the UI to bleed into your view models and models, so that you can raise on the proper thread, which in turn prevents you from using INPC from a .NET Standard library (unless you want to get into bait'n'switch).
In addition you might have properties changing multiple times during one frame, so you could potentially be jumping to the UI Thread many more times than are really needed (all that's needed is to flag the property as "dirty", and be picked up on the next render pass.
All this causes significantly extra code complexity and is more error prone.
It also causes an issue in WinUI 3.0, because we now have two UI Threads: Legacy UWP, and WinUI threads: Which one should I be raising on?
Scope
Important Notes
Threading concerns are of course valid, but I don't believe this is really a problem in the end. It doesn't matter that multiple INPC events fires before the UI updates, as only the last one would ever matter. This would be no different than existing code switching to the UI Thread multiple times. Once the first one switches to the UI Thread, the property could already have updated many times already, and thus last one in wins. Naturally there would need to be a short lock when the value is read that prevents the dirty-flag from being set, so a second round of UI updates happen on the next frame, should the property be updated the same time it's being read. In fact if a value for instance changes from false, to true, and back to false, the Dependency Property is already smart enough to detect that the value didn't change, and can optimize by not causing a new rendering pass due to the property non-change.