[Bug]: Duplicate deactivation of ViewModel, from ViewModelActivator. about reactiveui HOT 4 OPEN

Do you need to dispose is the first reaction. Observables often do not.

from reactiveui.

Strictly speaking, probably not. .Deactivate(ignoreRefCount: true) should force the removal of the sensitive references that could cause a memory leak if left alive.

This still seems like a violation of the class's apparent contract. And to me, there's value in Disposing the activator, in that if something DOES try to activate it again, I get an exception, and a bug in my own code is revealed.

from reactiveui.

Observables aren't honoring that contract anyway. They just adopted the IDisposable to get advantages of some of the mechanics. It's a mistake to think you have to dispose all of them. Depends on ownership of the handle.

from reactiveui.

I meant more the "contract" regarding the Activated and Deactivated events. That's got nothing to do with disposal mechanics.

Actually, scratch what I said earlier. ViewModelActivator.Deactivate(ignoreRefCount: true) does not clear out all potentially sensitive references, because the two subjects for Activated and Deactivated could have subscribers, and the only way to clear those, currently, is through disposal.

I think an appropriate change would be from...

public void Deactivate(bool ignoreRefCount = false)
{
    if (Interlocked.Decrement(ref _refCount) == 0 || ignoreRefCount)
    {
        Interlocked.Exchange(ref _activationHandle, Disposable.Empty).Dispose();
        _deactivated.OnNext(Unit.Default);
    }
}

...to...

public void Deactivate(bool ignoreRefCount = false)
{
    var wasActive = ignoreRefCount
        ? (Interlocked.Exchange(ref _refCount, 0) != 0)
        : (Interlocked.Decrement(ref _refCount) == 0);

    if(wasActive)
    {
        Interlocked.Exchange(ref _activationHandle, Disposable.Empty).Dispose();
        _deactivated.OnNext(Unit.Default);
    }
}

from reactiveui.