-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[Bug]: Property observers on the same object, but with different schedulers, sometimes run on the wrong scheduler #3621
Comments
Thank you for reporting this, we will take a look as soon as possible to see how best to resolve this. |
Whats your You need to make sure you set WPF .NET based applications to a minimum of This is due to downstream dependencies requiring a minimum or net6.0-windows10.0.19041 |
@glennawatson We are targeting |
Target You can still run on older SDK versions you just get bug fixes with the newer SDK System.Reactive has that minimum target (which is not something we control in ReactiveUI) |
This is again by-design, ViewModels inherit the thread affinity of their views - meaning that once you bind a ViewModel, you should only set its properties on the UI thread. ReactiveUI intentionally does not solve this problem because if it did, it would cause threading "convoys" where scheduling delays propagate and build up in large applications. Instead of: // On some other thread
someViewModel.Foo = "Bar"; Do: // On some other thread
RxApp.MainThreadScheduler.Schedule(() => someViewModel.Foo = "Bar"); Basically the short version is, writing |
Howdy, @anaisbetts could we get the requirement for viewmodel properties being updated on the UIThread documented on the databindings page or maybe somewhere related to viewmodel properties? I think it needs to be documented because in vanilla WPF (I assume other XAML framework too), bindings made in XAML handle other threads updating a viewmodel property and there isn't even a trace log message about it being remotely problematic. I think the requirement has good intentions, but it is surprising when you hit it coming from vanilla and annoying that there is no workaround to get a scheduler in there given that some legacy viewmodel code may be written with the vanilla assumption. Adding insult to injury in most cases(maybe all cases by design?) My final thought about this is that having to add |
Describe the bug 🐞
If an object has multiple properties that observe another object with
WhenAnyValue().ToProperty()
, and some (but not all) of them useObserveOn
to ensure they are updated on the UI thread, they are sometimes updated on background threads anyway.I think the minimal scenario is:
WhenAnyValue(...).ObserveOn(RxApp.MainThreadScheduler).ToProperty()
.ObserveOn
(or uses it with some other scheduler).If the business object's property change events fire too rapidly, the binding will try to update the view on a background thread, which causes a crash under WPF.
Step to reproduce
Run this test.
ObserveOnTests.cs.txt
Case 2, where
Counter2
observes the business object onCurrentThreadScheduler
, will fail somewhere in the first few hundred iterations.(The same behavior happens if the scheduler is passed to
ToProperty
.)Reproduction repository
https://github.com/reactiveui/ReactiveUI
Expected behavior
I expect that a property defined with
ObserveOn(MainThreadScheduler).ToProperty()
will always update on the main thread.Screenshots 🖼️
No response
IDE
No response
Operating system
Windows 11
Version
No response
Device
No response
ReactiveUI Version
18.3.1 and 19.4.1
Additional information ℹ️
The real case where this came up is in a WPF app that uses a mix of ReactiveUI bindings (to view model properties that marshal to the UI thread) and XAML bindings (to properties that don't marshal, because that's allowed). I'm aware this is not the recommended way to build view models.
The text was updated successfully, but these errors were encountered: