WhenAnyObservable doesn't unsubscribe on child null.

[grokys]

I'm not sure if this is intended behaviour or not, but if an observable on a child object is subscribed to using WhenAnyObservable and then the child object is removed, the observable continues to fire.

The following code demonstrates this:

using System;
using System.Reactive.Subjects;
using ReactiveUI;

namespace ReactiveUI_WhenAnyObservable_Bug
{
    class Program
    {
        static void Main(string[] args)
        {
            var container = new Container();
            container.WhenAnyObservable(x => x.Child.Foo).Subscribe(i => Console.WriteLine(i));

            var child = new Child();
            container.Child = child;
            child.Foo.OnNext(1);
            container.Child = null;
            child.Foo.OnNext(2);
        }
    }

    class Container : ReactiveObject
    {
        private Child child;

        public Child Child
        {
            get { return child; }
            set { this.RaiseAndSetIfChanged(ref child, value); }
        }
    }

    class Child
    {
        public Child()
        {
            Foo = new Subject<int>();
        }

        public Subject<int> Foo { get; private set; }
    }
}

If you run this, you'll see both 1 and 2 output, despite by the time child.Foo.OnNext(2) was called, child is no longer a child of container

Is this a bug? If not, is there a work-around?

[anaisbetts]

This is definitely a bug

[grokys]

Any change of this getting fixed? Or if you could point me in the right direction I could take a look.

[anaisbetts]

@grokys You can have a look at the implementation of WhenAnyObservable, it's pretty straightforward. You can probably write an alternate one in your app, it doesn't require any private methods

[kentcb]

FYI, the problem is actually in ObservableForProperty. It's not notifying in this scenario:

        [Fact]
        public void NAME_THIS_SUCKER()
        {
            var fixture = new TestWhenAnyObsViewModel();
            var output = fixture.ObservableForProperty(x => x.Child.MyListOfInts).CreateCollection();

            Assert.Equal(0, output.Count);

            fixture.Child = new TestWhenAnyObsViewModel
            {
                MyListOfInts = new ReactiveList<int>()
            };

            Assert.Equal(1, output.Count);

            fixture.Child = null;
            Assert.Equal(2, output.Count);
        }

The final assertion is failing because there is 1 item in output, not 2.

This is in turn affecting WhenAnyObservable because it never knows the container's child was set to null so it doesn't know to disconnect (and if it did receive the notification it will currently fail anyway until PR #831 is applied).

The cause is down to this clause in ReactiveNotifyPropertyChangedMixin.SubscribeToExpressionChain:

notifier = notifier.Where(x => x.Sender != null);

Sender is null for some reason. I'll try to dig into why tomorrow.

[kentcb]

Here's my best articulation of the problem based on some further research into this issue...

In the example test in my previous post, when fixture.Child is set to null, the observable monitoring the MyListOfInts portion of the entire expression is fired with a source change of:

• Expression: x.Child
• Sender: TestWhenAnyObsViewModel
• Value: null

because the observedChangeFor method is attempting to translate this source change into another source change relative to the passed in expression (i.e. relative to MyListOfInts), it wants the sender to be the object that owns the ReactiveList<int> returned by MyListOfInts. However, since that object was just set to null, it has no way of determining the sender. Therefore, the sender is set to null and thus this particular notification is filtered out of the reactive pipeline returned by SubscribeToExpressionChain.

I need to work on something else right now, but it seems to me the only way forward here is for IObservedChange<TSender, TValue> to also contain the previous value. That way, it would be possible for observedChangeFor to accurately determine the sender where a parent property is set to null. This would have the added benefit of making IObservedChange more useful in general.

Any thoughts on this? I'd be happy to take a stab at a PR when I get a chance, but I want to make sure I'm not off base with my thinking here first.

[grokys]

Thanks for taking a look Kent. I've tried delving into it myself but I'll need more time to familiarize myself with how it all works. The minimal repro you posted will certainly help though.

[kentcb]

Reviewing this again, I think this is probably just down to the semantics of WhenAny* with respect to invalid property chains. By breaking the chain, it keeps ahold of the last known value (it doesn't get replaced with null like I used to expect). This has come up before in #314 and #976. I'm still not convinced the behavior is correct, but have reached the point where I've just accepted it for what it is :)

Pinging @paulcbetts to see if he agrees.

[csmager]

I'm currently struggling with this. Say I have an app that has the idea of opening a 'project'. When a project is open, its name and save state is shown in the title bar. When no project is open, no title should be shown. I have an expression like this:

this.WhenAny(x => x.Project.Name, x => x.Project.HasUnsavedChanges,
        (name, hasChanges) => hasChanges.Value ? $"{name.Value}*" : name.Value)
    .ToProperty(this, x => x.Title, out _title);

When Project is set to null, I get the old 'Title` value hanging around until another is opened. My current workaround is to merge with this:

this.WhenAnyValue(x => x.Project).Where(x => x == null).Select(_ => string.Empty);

I see from the other linked issues that this is 'by design', but I really don't see that this is desirable or intuitive. If we really believe this is a good default, it'd be good to able to opt-in to a behaviour where we get default(T) when one of the properties in the chain is null.

It could be me though. Am I approaching this the wrong way? I feel I'm having to fight ReactiveUI a lot today.

[pr8x]

Any update on this issue? Subscribing to a nested chain of properties where a part may be null is pretty common thing to do. I am surprised the behaviour is so weird in ReactiveUI.

[brignolff]

Hi,

Is there any news on this bug ? Got the same problem and would love to have an answer.