ReactiveUI
ReactiveUI copied to clipboard
reactiveui
[Bug]: Property observers on the same object, but with different schedulers, sometimes run on the wrong scheduler
Describe the bug 🐞
If an object has multiple properties that observe another object with WhenAnyValue().ToProperty(), and some (but not all) of them use ObserveOn to ensure they are updated on the UI thread, they are sometimes updated on background threads anyway.
I think the minimal scenario is:
- A ReactiveObject ("business object") has one or more properties which get updated by a worker thread.
- Another ReactiveObject ("view model") has a property P1 which is linked to the business object by
WhenAnyValue(...).ObserveOn(RxApp.MainThreadScheduler).ToProperty(). - A third object ("view") observes P1, and its observer needs to run on the main thread.
- But the view model also has a property P2 which is linked to the same business object, and does not use
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.
Case 2, where Counter2 observes the business object on CurrentThreadScheduler, 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.
Thank you for reporting this, we will take a look as soon as possible to see how best to resolve this.
Whats your RxApp.MainThreadScheduler type when you look at it.
You need to make sure you set WPF .NET based applications to a minimum of net6.0-windows10.0.19041 otherwise the MainThreadScheduler can be set to the incorrect object.
This is due to downstream dependencies requiring a minimum or net6.0-windows10.0.19041
@glennawatson We are targeting net7.0-windows. RxApp.MainThreadScheduler is type ReactiveUI.WaitForDispatcherScheduler.
Target net7.0-windows10.0.19041
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 WhenAnyValue.ObserveOn is always a bug. Instead you need to track down the code doing the Setting and fix That instead.
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?) BindTo is being called from a view where it could easily access the UI thread Dispatcher to do the scheduling. But I realize that probably can't be done in a single portable library and instead would need implementations for each framework.
My final thought about this is that having to add .ObserveOn() wherever viewmodel properties are set sounds like a mess.
