reactiveui / Sextant

A ReactiveUI navigation library for Xamarin.Forms
MIT License
158 stars 26 forks source link

[BUG] When binding received parameter in ContentPage from INavigable ViewModel exception is thrown that cannot access UI control from another thread #136

Open xplatsolutions opened 5 years ago

xplatsolutions commented 5 years ago

This is reproducible from Sample project, in Android but I was able to make it happen in iOS as well.

  1. Create a ViewModel implementing INavigable

    
    public class ReceivedViewModel : NavigableViewModel
    {
        public ReceivedViewModel(
                IScheduler mainThreadScheduler = null,
                IScheduler taskPoolScheduler = null)
            : base("Second Screen", mainThreadScheduler, taskPoolScheduler)
        {
        }
    
        public override string Id => "Received";
    
        [Reactive]
        public string ReceivedParameter { get; set; }
    
        public override IObservable<Unit> WhenNavigatingTo(INavigationParameter parameter)
        {
            if (parameter.ContainsKey("parameter"))
            {
                var received = parameter["parameter"];
                ReceivedParameter = received.ToString();
            }
    
            return base.WhenNavigatedTo(parameter);
        }
    }

public abstract class NavigableViewModel : ViewModelBase, INavigable { protected NavigableViewModel( string title, IScheduler mainThreadScheduler = null, IScheduler taskPoolScheduler = null) : base( title, mainThreadScheduler, taskPoolScheduler) { }

    public virtual string Id { get; }

    public virtual IObservable<Unit> WhenNavigatedFrom(INavigationParameter parameter) => Observable.Return(Unit.Default);

    public virtual IObservable<Unit> WhenNavigatedTo(INavigationParameter parameter) => Observable.Return(Unit.Default);

    public virtual IObservable<Unit> WhenNavigatingTo(INavigationParameter parameter) => Observable.Return(Unit.Default);
}

///

/// A base for all the different view models used throughout the application. /// public abstract class ViewModelBase : ReactiveObject, IActivatableViewModel { /// /// Initializes a new instance of the class. /// /// The title of the view model for routing purposes. /// The scheduler to use to schedule operations on the main thread. /// The scheduler to use to schedule operations on the task pool. protected ViewModelBase( string title, IScheduler mainThreadScheduler = null, IScheduler taskPoolScheduler = null) {

        // Set the schedulers like this so we can inject the test scheduler later on when doing VM unit tests
        MainThreadScheduler = mainThreadScheduler ?? RxApp.MainThreadScheduler;
        TaskPoolScheduler = taskPoolScheduler ?? RxApp.TaskpoolScheduler;

        ShowAlert = new Interaction<AlertViewModel, Unit>(MainThreadScheduler);
        OpenBrowser = new Interaction<string, Unit>(MainThreadScheduler);
    }

    /// <summary>
    /// Gets the activator which contains context information for use in activation of the view model.
    /// </summary>
    public ViewModelActivator Activator { get; } = new ViewModelActivator();

    /// <summary>
    /// Gets a interaction which will show an alert.
    /// </summary>
    public Interaction<AlertViewModel, Unit> ShowAlert { get; }

    /// <summary>
    /// Gets an interaction which will open a browser window.
    /// </summary>
    public Interaction<string, Unit> OpenBrowser { get; }

    /// <summary>
    /// Gets the scheduler for scheduling operations on the main thread.
    /// </summary>
    protected IScheduler MainThreadScheduler { get; }

    /// <summary>
    /// Gets the scheduler for scheduling operations on the task pool.
    /// </summary>
    protected IScheduler TaskPoolScheduler { get; }
}

2. In the respective ContentPage add binding.

public partial class ReceivedView : ContentPageBase { public ReceivedView() { InitializeComponent();

        // Setup the bindings.
        // Note: We have to use WhenActivated here, since we need to dispose the
        // bindings on XAML-based platforms, or else the bindings leak memory.
        this.WhenActivated(disposable =>
        {
            // This line throws an error.
            this.OneWayBind(this.ViewModel, vm => vm.ReceivedParameter, view => view.ReceivedParameter.Text).DisposeWith(ControlBindings);
        });
    }
}

///

/// A base page used for all our content pages. It is mainly used for interaction registrations. /// /// The view model which the page contains. public class ContentPageBase : ReactiveContentPage where TViewModel : ViewModelBase { protected CompositeDisposable ControlBindings { get; } = new CompositeDisposable();

    /// <summary>
    /// Initializes a new instance of the <see cref="ContentPageBase{TViewModel}"/> class.
    /// </summary>
    public ContentPageBase()
    {
        this.WhenActivated(disposables =>
        {
            ViewModel
                .ShowAlert
                .RegisterHandler(interaction =>
                {
                    AlertViewModel input = interaction.Input;
                    DisplayAlert(input.Title, input.Description, input.ButtonText);
                    interaction.SetOutput(Unit.Default);
                })
                .DisposeWith(disposables);

            ViewModel
                .OpenBrowser
                .RegisterHandler(interaction =>
                {
                    Device.OpenUri(new Uri(interaction.Input));
                    interaction.SetOutput(Unit.Default);
                })
                .DisposeWith(disposables);
        });
    }
}

<?xml version="1.0" encoding="UTF-8" ?> <ui:ContentPageBase xmlns="http://xamarin.com/schemas/2014/forms" xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" xmlns:local="clr-namespace:Eight.Influencers.Views" x:Class="Eight.Influencers.Views.ReceivedView" xmlns:rxui="clr-namespace:ReactiveUI.XamForms;assembly=ReactiveUI.XamForms" xmlns:ui="clr-namespace:Eight.Influencers.Views" xmlns:vms="clr-namespace:Eight.Influencers.ViewModels;assembly=Eight.Influencers.ViewModels" x:TypeArguments="vms:ReceivedViewModel">

``` 3. Create a ViewModel to navigate to the INavigable page ``` public class MainViewModel : NavigableViewModel, ISupportsValidation { public MainViewModel( IScheduler mainThreadScheduler = null, IScheduler taskPoolScheduler = null, IParameterViewStackService parameterViewStackService = null) : base("Main", mainThreadScheduler, taskPoolScheduler) { var navigationService = parameterViewStackService ?? Locator .Current .GetService(); Navigate = ReactiveCommand .CreateFromObservable( () => navigationService .PushPage(new ReceivedViewModel(), new NavigationParameter { { "parameter", PassingParameter } }), ValidationContext.Valid, RxApp.MainThreadScheduler); this.ValidationRule( viewModel => viewModel.PassingParameter, parameter => !string.IsNullOrWhiteSpace(parameter) && int.TryParse(parameter, out int result), "You must specify a number."); } public override string Id => nameof(MainViewModel); public ReactiveCommand Navigate { get; } public string PassingParameter => "1"; public ValidationContext ValidationContext { get; } = new ValidationContext(); } ``` 4. Run the project, in WhenActivated delegate the binding will throw an error. If you use Xamarin XAML Binding it work properly. `Text="{Binding ReceivedParameter}"` **Expected behavior** Expected to update the label with no error and show the received parameter value. **Environment** - OS: Simulator XR iOS 12.4 - Version Sextant.XamForms 2.2.1 Exception Stacktrace. ``` > PropertyBinderImplementation: view.ReceivedParameter.Text Binding received an Exception! - System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> UIKit.UIKitThreadAccessException: UIKit Consistency error: you are calling a UIKit method that can only be invoked from the UI thread. > at UIKit.UIApplication.EnsureUIThread () [0x0001a] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.14.0.114/src/Xamarin.iOS/UIKit/UIApplication.cs:95 > at UIKit.UILabel.set_Text (System.String value) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.14.0.114/src/Xamarin.iOS/UIKit/UILabel.g.cs:660 > at Xamarin.Forms.Platform.iOS.LabelRenderer.UpdateText () [0x00066] in :0 > at Xamarin.Forms.Platform.iOS.LabelRenderer.OnElementPropertyChanged (System.Object sender, System.ComponentModel.PropertyChangedEventArgs e) [0x00097] in :0 > at (wrapper delegate-invoke) .invoke_void_object_PropertyChangedEventArgs(object,System.ComponentModel.PropertyChangedEventArgs) > at Xamarin.Forms.BindableObject.OnPropertyChanged (System.String propertyName) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:211 > at Xamarin.Forms.Element.OnPropertyChanged (System.String propertyName) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\Element.cs:359 > at Xamarin.Forms.BindableObject.SetValueActual (Xamarin.Forms.BindableProperty property, Xamarin.Forms.BindableObject+BindablePropertyContext context, System.Object value, System.Boolean currentlyApplying, Xamarin.Forms.Internals.SetValueFlags attributes, System.Boolean silent) [0x00114] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:443 > at Xamarin.Forms.BindableObject.SetValueCore (Xamarin.Forms.BindableProperty property, System.Object value, Xamarin.Forms.Internals.SetValueFlags attributes, Xamarin.Forms.BindableObject+SetValuePrivateFlags privateAttributes) [0x00173] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:379 > at Xamarin.Forms.BindableObject.SetValue (Xamarin.Forms.BindableProperty property, System.Object value, System.Boolean fromStyle, System.Boolean checkAccess) [0x00042] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:316 > at Xamarin.Forms.BindableObject.SetValue (Xamarin.Forms.BindableProperty property, System.Object value) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\BindableObject.cs:293 > at Xamarin.Forms.Label.set_Text (System.String value) [0x00000] in D:\a\1\s\Xamarin.Forms.Core\Label.cs:132 > at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&) > at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0006a] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.14.0.114/src/Xamarin.iOS/mcs/class/corlib/System.Reflection/RuntimeMethodInfo.cs:391 > --- End of inner exception stack trace --- > at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00081] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.14.0.114/src/Xamarin.iOS/mcs/class/corlib/System.Reflection/RuntimeMethodInfo.cs:401 > at System.Reflection.RuntimePropertyInfo.SetValue (System.Object obj, System.Object value, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] index, System.Globalization.CultureInfo culture) [0x0005d] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.14.0.114/src/Xamarin.iOS/mcs/class/corlib/System.Reflection/RuntimePropertyInfo.cs:443 > at System.Reflection.PropertyInfo.SetValue (System.Object obj, System.Object value, System.Object[] index) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.14.0.114/src/Xamarin.iOS/external/corefx/src/Common/src/CoreLib/System/Reflection/PropertyInfo.cs:55 > at ReactiveUI.PropertyBinderImplementation+<>c__DisplayClass10_0`3[TTarget,TValue,TObs].g__SetThenGet|0 (System.Object paramTarget, System.Object paramValue, System.Object[] paramParams) [0x00020] in <0154c0d4daaf4d26995e5d3fda8fad1e>:0 > at ReactiveUI.PropertyBinderImplementation+<>c__DisplayClass10_0`3[TTarget,TValue,TObs].b__7 (<>f__AnonymousType0`2[j__TPar,j__TPar] x) [0x0001d] in <0154c0d4daaf4d26995e5d3fda8fad1e>:0 > at System.Reactive.Linq.ObservableImpl.Select`2+Selector+_[TSource,TResult].OnNext (TSource value) [0x00008] in :0 ```
open-collective-bot[bot] commented 5 years ago

Hey @xplatsolutions :wave:,

Thank you for opening an issue. We will get back to you as soon as we can. Also, check out our Open Collective and consider contributing financially.

https://opencollective.com/reactiveui

PS.: We offer priority support for all financial contributors. Don't forget to add priority label once you start contributing :smile:

winterdouglas commented 4 years ago

I couldn't reproduce the exact same issue as is described here, it doesn't crash with the version 2.2.1. For the test I used Forms 4.4, RxUI 11. Tried the latest Sextant 2.4.1 as well, doesn't crash either. However, there are a few remarks:

  1. On iPhone XR iOS 12.4 the BindValidation fails with an IndexOutOfRangeException. The same doesn't happen if I run the exact same code on iOS 13.
  2. The WhenNavigatingTo runs out of the main thread (should it? @RLittlesII), and then the property is set as is, which makes it to not update the UI on iOS. It doesn't crash for me, just the value that doesn't get displayed in the view.
  3. If I do force the property to be set on the main thread, everything works well (except that BindValidation above, I had to comment it out to run on iOS 12.4).
RLittlesII commented 4 years ago

@winterdouglas It should execute on the main thread, or whatever thread the View believes is the main thread. Although looking at it now and remembering an issue I had a few weeks ago, this is most likely not happening on the main thread.

https://github.com/reactiveui/Sextant/blob/b4fda7802e761447db26e42c3553a37a3f919a4a/src/Sextant/Navigation/ParameterViewStackService.cs#L54-L65

gsgou commented 3 years ago

i reproduce the same issue on: Device: iPhone 6S 12.4 Version: Sextant.XamForms 2.10.1