reactiveui / ReactiveUI

An advanced, composable, functional reactive model-view-viewmodel framework for all .NET platforms that is inspired by functional reactive programming. ReactiveUI allows you to abstract mutable state away from your user interfaces, express the idea around a feature in one readable place and improve the testability of your application.
https://www.reactiveui.net
MIT License
8.06k stars 1.12k forks source link

[Bug] Alternative STA threads do not play nice with WhenAnyValue and dependency properties. #1375

Open bradphelan opened 7 years ago

bradphelan commented 7 years ago

What is the current behavior?

<TextBox x:Name="FilterText"/>

and

this.WhenAnyValue(p=>p.FilterText.Text).Subscribe(Console.WriteLine)

will crash with a cross thread access exception. This happens if the window the control in hosted in is running in a different thread than the RxApp.MainSchedular

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem

Full description is here

https://stackoverflow.com/questions/44236317/whenanyvalue-throws-a-threading-exception-when-used-against-a-dependency-propert/44237800#44237800

What is the expected behavior?

When WhenAnyValue is executed in a specific synchronization context then it should not blindly dispatch to RxApp.MainThreadScheduler.

What is the motivation / use case for changing the behavior?

My logging window does't work :(

Which versions of ReactiveUI, and which platform / OS are affected by this issue? Did this work in previous versions of ReativeUI? Please also test with the latest stable and snapshot (http://docs.reactiveui.net/en/contributing/snapshot/index.html) versions.

Currently running 7.2

Other information (e.g. stacktraces, related issues, suggestions how to fix)

https://stackoverflow.com/questions/44236317/whenanyvalue-throws-a-threading-exception-when-used-against-a-dependency-propert/44237800#44237800

rdavisau commented 7 years ago

I wasn't able to reproduce this in a brand new WPF project. Is there any additional complexity in the way you call CreateAndShowStaWindow or in your factory?

I used:

    public partial class MainWindow : Window
    {
        public MainWindow()
        {
            InitializeComponent();
        }

        private async void Button_Click(object sender, RoutedEventArgs e)
        {
            await CreateAndShowStaWindow(() => new Window1());
        }

        public static Task<T> CreateAndShowStaWindow<T>(Func<T> factory) where T : Window
        {
            var windowResult = new TaskCompletionSource<T>();

            var newWindowThread = new Thread(() =>
            {
                var window = factory();
                window.Show();
                windowResult.SetResult(window);            
                window.Events().Closed.Subscribe( _ => window.Dispatcher.InvokeShutdown() );
                System.Windows.Threading.Dispatcher.Run();

            });
            newWindowThread.SetApartmentState(ApartmentState.STA);
            newWindowThread.IsBackground = true;
            newWindowThread.Start();
            newWindowThread.Name = "STA WPF";

            return windowResult.Task;
        }
    }
    public partial class Window1 : Window
    {
        public Window1()
        {
            InitializeComponent();

            this.WhenAnyValue(x => x.FilterText.Text).Subscribe(x => Console.WriteLine(x));
        }
    }
bradphelan commented 7 years ago

Yeah. I just did the same and the error does not occur. Something evil going on :(

bradphelan commented 7 years ago

I'll try and isolate some more features of my large project in the test project and get back to you soon.

bradphelan commented 7 years ago

I can't figure it out. Is there anything in the code that might try to dispatch to another context under certain conditions?

bradphelan commented 7 years ago

Got it.

Add in the following calls

       Locator.CurrentMutable.InitializeReactiveUI();

I have a reproducing repository.

https://github.com/bradphelan/RxUIBug1375

There is probably a bit more guff in there than necessary to reproduce as I was adding in more and more stuff till I found the trigger. I'll try to pare it back a bit.

bradphelan commented 7 years ago

The reason I was calling InitializeReactiveUI is made clearer by the following snippet from my main application. I had to set up the RxUI / Ninject binding as well as some other stuff. The code has been there for a long time and works apart from this discovery I have made now.

    public static void Init(IKernel kernel)
    {
        var customResolver = new FuncDependencyResolver
            (
             (service, contract) =>
             {
                 if (contract != null)
                     return kernel.GetAll( service, contract );
                 var items = kernel.GetAll( service );
                 var list = items.ToList();
                 return list;
             }
             , (factory, service, contract) =>
             {
                 var binding = kernel.Bind( service ).ToMethod( _ => factory() );
                 if (contract != null)
                     binding.Named( contract );
             } );

        Locator.CurrentMutable.InitializeSplat();
        Locator.CurrentMutable.InitializeReactiveUI();
        Locator.CurrentMutable = customResolver;
        Locator.CurrentMutable.RegisterLazySingleton(() => new WeinCadViewLocator(kernel), typeof(IViewLocator));

        var log = SerilogExtensions.DefaultLogger;
        Log.Logger = log;
        SerilogSplatLogger.Register(log);
    }
stale[bot] commented 7 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this because it helps reduce the workload of our extremely busy maintainers. If you want this issue progressed faster please start talks with a maintainer with how you can help out. There are so many ways to contribute to open-source and the most valued are often not code related. We are always looking for more help and look after those who stick around. Say howdy to one of the maintainers or browse https://reactiveui.net/contribute/ for ideas on where to start your journey.

JKronberger commented 7 years ago

This should not be marked as stale as there is a reproducing example provided

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this because it helps reduce the workload of our extremely busy maintainers. If you want this issue progressed faster please start talks with a maintainer with how you can help out. There are so many ways to contribute to open-source and the most valued are often not code related. We are always looking for more help and look after those who stick around. Say howdy to one of the maintainers or browse https://reactiveui.net/contribute/ for ideas on where to start your journey.