[Bug]: ViewModelActivator.Deactivate() allows negative refCount when called without prior Activate()

[zhaparoff]

Describe the bug 🐞

ViewModelActivator.Deactivate() does not validate that _refCount > 0 before decrementing, which allows the reference count to become negative when Deactivate() is called more times than Activate(). This can lead to ViewModels that can never be properly activated again.

Consequences

1. Silent failures: ViewModels appear to work but subscriptions never fire
2. Difficult to debug: No exception thrown, ViewModel just doesn't work
3. Data binding issues: UI won't update because WhenActivated blocks never execute
4. Reference count corruption: Once negative, the ViewModel is permanently broken

Step to reproduce

Environment

• ReactiveUI version: 22.3.1
• Platform: WPF (.NET 8) (but I believe it works in the same for all platforms)

Sample repro code

var viewModel = new MyViewModel(); // implements IActivatableViewModel
var activationCount = 0;

viewModel.WhenActivated(disposables =>
{
    activationCount++;
    Disposable.Create(() => activationCount--).DisposeWith(disposables);
});

// Call Deactivate WITHOUT calling Activate first
viewModel.Activator.Deactivate();

// Now try to activate
viewModel.Activator.Activate();

// Expected: activationCount == 1
// Actual: activationCount == 0 (ViewModel never activated)

Actual Scenario Where This Occurs

In a parent-child ViewModel scenario:

public class ParentViewModel : ReactiveViewModel
{
    private IChildViewModel _currentChild;
    
    public IChildViewModel CurrentChild
    {
        set
        {
            // Problem: Deactivates child even if parent isn't active yet
            _currentChild?.Activator.Deactivate();  
            _currentChild = value;
            _currentChild?.Activator.Activate();
        }
    }
    
    public ParentViewModel(IChildViewModel child1, IChildViewModel child2)
    {
        _currentChild = child1;  // Set initial child BEFORE parent is activated
    }
    
    protected override async Task Activate(...)
    {
        await base.Activate(...);
        // Later, property changes trigger child swap
        // CurrentChild = child2;  
        // ↑ This deactivates child1 (refCount: 0 → -1)
        //   Then activates child2 (refCount: 0 → 1) ✓
        // CurrentChild = child1;
        // ↑ Deactivates child2 (refCount: 1 → 0) ✓
        //   Tries to activate child1 (refCount: -1 → 0) ✗ BROKEN!
    }
}

Expected behavior

Ideally, logic should not allow refCount to drop below zero.

Option 1 (Guard against misuse):

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

Option 2 (Throw exception to catch misuse):

public void Deactivate(bool ignoreRefCount = false)
{
    var newCount = Interlocked.Decrement(ref _refCount);
    
    if (newCount < 0)
    {
        Interlocked.Increment(ref _refCount); // Restore
        throw new InvalidOperationException(
            "Deactivate called more times than Activate. This indicates a bug in activation lifecycle management.");
    }
    
    if (newCount == 0 || ignoreRefCount)
    {
        Interlocked.Exchange(ref _activationHandle, Disposable.Empty).Dispose();
        _deactivated.OnNext(Unit.Default);
    }
}

ReactiveUI Version

22.3.1

Additional information ℹ️

Root Cause

In ViewModelActivator.Deactivate():

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

Interlocked.Decrement executes unconditionally, so:

• Initial state: _refCount = 0
• Call Deactivate(): _refCount = -1 (condition false, no deactivation fires)
• Call Activate(): _refCount = 0 (condition false, no activation fires)

The ViewModel is now in a corrupted state where it can never activate.

Workaround

Without fix application code must guard deactivation all the time:

// Instead of:
childViewModel.Activator.Deactivate();

// Use:
if (IsActivated)  // Track and check parent state
{
    childViewModel.Activator.Deactivate(ignoreRefCount: true);
}

Additional Context

No existing unit tests verify behavior when Deactivate() is called without prior Activate(). All tests in https://github.com/reactiveui/reactiveui/tree/main/src/tests/ReactiveUI.Tests/Activation/ViewModelActivatorTests.cs follow the correct pattern of activating before deactivating.

Possibly have the same root cause with #3635

[glennawatson]

This has been fixed in #4301.

The root cause was that Deactivate() unconditionally called Interlocked.Decrement on the reference count without guarding against it already being zero. If Deactivate() was called more times than Activate(), the ref count went negative, which prevented future Activate() calls from triggering activation (since incrementing from -1 yields 0, not 1).

The fix replaces the decrement with a CAS (CompareExchange) loop that refuses to go below zero. The ignoreRefCount path now explicitly resets the count to 0.

This should be available in the next release.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.