ObserveCountChanged on a view: a count change during Subscribe is silently dropped, as in #132

Read ObservableCollectionR3Extensions.View.cs around lines 405, 418, and 490, then compare the subscription locking used by ToViewList and ToNotifyCollectionChanged. Reproduce the race with the provided RunOnAttach example, including the filter reset case. Done means a reset during subscription reliably emits the count change, with the initial notification behavior considered alongside the deadlock concern described in the issue.

The bug in #132 also exists in the overload of ObserveCountChanged for ISynchronizedView<T, TView>. #140 fixes only the overload for IObservableCollection<T>.

If a Reset that leaves the view empty (clearing the source, or attaching a filter that rejects every item) happens while the subscription is still being established, the subscriber is never told that the count changed. Nothing is logged and no exception is raised. With notifyCurrentCount: true, the same window also produces a duplicated or out-of-order first value.

Why it happens

The order is the same as in #132. The base constructor attaches the handler, and the derived constructor initializes countPrev after that. Neither step is inside lock (SyncRoot).

// base ctor: SynchronizedViewObserverBase<T, TView, TEvent>
source.ViewChanged += handlerDelegate;   // (1) runs first

// derived ctor: _SynchronizedViewCountChanged
this.countPrev = source.Count;           // (2) runs after (1)
if (notifyCurrentCount) observer.OnNext(source.Count);

• attach: ObservableCollectionR3Extensions.View.cs#L490
• initialization: #L405
• the guard that reads it: #L418

A Reset raised in that window sees countPrev still at its default 0, so the guard countPrev != source.Count evaluates 0 != 0 and drops the change. Step (2) then stores 0, so nothing is left to detect it afterwards.

Repro

The decorator from #132, adapted to ISynchronizedView<T, TView>, gives a point to run code right after the handler is attached:

sealed class RunOnAttach<T, TView>(ISynchronizedView<T, TView> inner, Action onAttach) : ISynchronizedView<T, TView>
{
    public object SyncRoot => inner.SyncRoot;
    public ISynchronizedViewFilter<T, TView> Filter => inner.Filter;
    public IEnumerable<(T Value, TView View)> Filtered => inner.Filtered;
    public IEnumerable<(T Value, TView View)> Unfiltered => inner.Unfiltered;
    public int UnfilteredCount => inner.UnfilteredCount;
    public int Count => inner.Count;

    public event NotifyViewChangedEventHandler<T, TView>? ViewChanged
    {
        add
        {
            inner.ViewChanged += value;
            onAttach();
        }
        remove => inner.ViewChanged -= value;
    }

    public event Action<RejectedViewChangedAction, int, int>? RejectedViewChanged
    {
        add => inner.RejectedViewChanged += value;
        remove => inner.RejectedViewChanged -= value;
    }

    public event Action<NotifyCollectionChangedAction>? CollectionStateChanged
    {
        add => inner.CollectionStateChanged += value;
        remove => inner.CollectionStateChanged -= value;
    }

    public void AttachFilter(ISynchronizedViewFilter<T, TView> filter) => inner.AttachFilter(filter);
    public void ResetFilter() => inner.ResetFilter();
    public ISynchronizedViewList<TView> ToViewList() => inner.ToViewList();
    public NotifyCollectionChangedSynchronizedViewList<TView> ToNotifyCollectionChanged() => inner.ToNotifyCollectionChanged();
    public NotifyCollectionChangedSynchronizedViewList<TView> ToNotifyCollectionChanged(ICollectionEventDispatcher? collectionEventDispatcher) => inner.ToNotifyCollectionChanged(collectionEventDispatcher);
    public IEnumerator<TView> GetEnumerator() => inner.GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
    public void Dispose() => inner.Dispose();
}

Another thread clears the list at that point. The subscribing thread waits until the clear has finished, or until it waits for a lock that the subscription holds, before it goes on:

var list = new ObservableList<int>(new[] { 1, 2, 3 });
using var view = list.CreateView(x => x);
var counts = new List<int>();

var clear = new Thread(() => list.Clear());
using var subscription = new RunOnAttach<int, int>(view, () =>
    {
        clear.Start();
        SpinWait.SpinUntil(() => (clear.ThreadState & (ThreadState.Stopped | ThreadState.WaitSleepJoin)) != 0);
    })
    .ObserveCountChanged()
    .Subscribe(counts.Add);
clear.Join();

// view.Count == 0
// counts:   {}          <- expected { 0 }

Running view.AttachFilter(x => false) on the other thread instead of list.Clear() drops the change the same way. Both were observed on master (a78ba19), on net8.0 and net10.0, and both pass with the fix below.

Suggested fix

Attach the handler and initialize countPrev inside lock (source.SyncRoot). Here source.SyncRoot is the lock of the view. Every view raises ViewChanged inside it, both for a change of the source and for AttachFilter and ResetFilter, so no change can come between the two steps. The view lists (ToViewList, ToNotifyCollectionChanged) already subscribe to a view inside that lock. This fixes the dropped change.

The fix relies on a view raising ViewChanged only inside its SyncRoot, and never while a handler is being attached. Every view in the library does so, and the README should state it as a requirement for implementations of ISynchronizedView<T, TView>. The same goes for IObservableCollection<T>, whose SyncRoot the fix in #140 relies on in the same way.

Send the initial value of notifyCurrentCount: true after releasing the lock, as today. The first value can then still be duplicated or out of order, and an out-of-order first value leaves the subscriber with a stale count until the next change.

Why not hold the lock through the initial notification

#140 holds the lock of the collection through the initial notification, which fixes the first value as well. For a view, that would run the first OnNext inside the lock of the view only. If the subscriber touches the source there (reading Count is enough) while another thread writes the source, the two threads take the two locks in opposite orders and deadlock, as in #145. The fix suggested in #145, taking the lock of the source first, does not apply here, because ISynchronizedView<T, TView> does not expose the lock of the source.

So the first value depends on how #145 is fixed. If a view comes to share the lock of its source, there is only one lock, and holding it through the initial notification no longer deadlocks this way. The first value can be fixed then.