Cysharp / Cysharp/ObservableCollections

ObserveCountChanged: a count change during Subscribe is silently dropped — the handler is attached before countPrev is initialized

Open
#132 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
1k
Forks
73
Avg merge
5d 4h
Merged PRs (30d)
2

Description

Subscribing to ObserveCountChanged (R3) while another thread is modifying the collection can lose a count change permanently.
If the collection is cleared while the subscription is still being established, the subscriber is never told that the count changed — it keeps reporting the count it saw at subscribe time until some later modification happens to correct it.
Nothing is logged, no exception is raised, and the observer's own state ends up self-consistent, so there is no way for either side to notice.
With notifyCurrentCount: true the same window also produces a duplicated or out-of-order first value.

The cause is an ordering problem in the subscription path, not in the collections themselves: the handler is attached before the observer has initialized the field it uses to decide whether the count changed, and neither step is inside lock (SyncRoot).

Why it happens

The attach happens in the base constructor, the initialization in the derived constructor, so C# initialization order makes the gap unavoidable:

// base ctor: ObservableCollectionObserverBase<T, TEvent>
collection.CollectionChanged += handlerDelegate;   // (1) runs first

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

If the collection is cleared in that window, the handler runs with countPrev still at its default 0, so the Reset guard countPrev != collection.Count evaluates 0 != 0 and the change is dropped.
Step (2) then stores 0, so nothing is left to detect it afterwards.

Repro

The window is a handful of instructions inside Subscribe, so a plain two-thread race cannot distinguish "the change was dropped" from "the clear simply finished before the subscription".
This decorator makes the same interleaving deterministic — it stands in for another thread that mutates the collection after += and before countPrev is set:

sealed class MutateOnAttach<T>(ObservableList<T> inner, Action<ObservableList<T>> mutate) : IObservableCollection<T>
{
    public int Count => inner.Count;
    public object SyncRoot => inner.SyncRoot;
    public ISynchronizedView<T, TView> CreateView<TView>(Func<T, TView> transform) => inner.CreateView(transform);
    public IEnumerator<T> GetEnumerator() => inner.GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

    public event NotifyCollectionChangedEventHandler<T>? CollectionChanged
    {
        add
        {
            inner.CollectionChanged += value;
            mutate(inner);           // the other thread wins the race here
        }
        remove => inner.CollectionChanged -= value;
    }
}
var inner = new ObservableList<int>(new[] { 1, 2, 3 });
var counts = new List<int>();

using var subscription = new MutateOnAttach<int>(inner, list => list.Clear())
    .ObserveCountChanged()
    .Subscribe(counts.Add);

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

The handler was attached when Reset was raised; it just could not tell that the count had changed, because its own idea of the previous count had not been initialized yet.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/ObservableCollections.R3/ObservableCollectionR3Extensions.cs at the attach site around line 567, then trace the initialization around line 427 and guard around line 440. Use the supplied MutateOnAttach repro to exercise the subscription race; done means a clear during subscription is reported as count 0 exactly once, with notifyCurrentCount preserving the expected ordering.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.