Reject collection modification from inside a `CollectionChanged` handler: reentrancy invalidates the event args for the other subscribers

Summary

IObservableCollection<T>.CollectionChanged is raised after the collection has already been
mutated, inside lock (SyncRoot), and nothing prevents a handler from mutating the collection
again. When one handler does, every handler that runs afterwards — including the remaining
handlers of the original notification — receives args describing a state the collection is no
longer in, delivered in the reverse of the order in which the changes actually happened.

I would like to propose that this be rejected outright, the way
ObservableCollection<T>.CheckReentrancy does.

Repro

var list = new ObservableList<int>(new[] { 1, 2, 3 });

// First handler: mutates the collection from inside the notification.
list.CollectionChanged += (in NotifyCollectionChangedEventArgs<int> e) =>
{
    if (e.Action == NotifyCollectionChangedAction.Add)
    {
        list.Clear();
    }
};

// Second handler: only reads the args it is given.
var received = new List<string>();
list.CollectionChanged += (in NotifyCollectionChangedEventArgs<int> e) =>
{
    received.Add(e.Action == NotifyCollectionChangedAction.Add
        ? $"Add {e.NewItem}@{e.NewStartingIndex} (Count={list.Count})"
        : $"{e.Action} (Count={list.Count})");
};

list.Add(4);

// received: { "Reset (Count=0)", "Add 4@3 (Count=0)" }

The second entry says "4 was added at index 3" to a collection that has no elements at all.
Nothing in the args lets the handler detect this, so a consumer cannot defend itself.

Why reentrancy is different from the other two ways args go stale

There are three ways a notification can end up describing a state the consumer no longer sees.
They are not equivalent, and only one of them is worth prohibiting:

	Notification order	Can the invariant be preserved?
Modification from another thread	Preserved — serialized by SyncRoot	Yes, already is
Deferred delivery via a dispatcher	Preserved — FIFO, only shifted in time	Yes — see #125
Reentrant modification	Reversed	No

The first two keep the causal order of the notifications, so a stateful consumer — which is what
a synchronized view is — can still apply them in sequence and arrive at the right content. Both
are also core scenarios for this library and cannot be given up.

The dispatcher case is worth spelling out, because it looks like this issue but is fixable. There
the args cannot be valid against the source: by the time the UI thread runs the handler, the
source has moved on. So the invariant has to be restated as "valid against the content the
consumer itself sees", and #125 is exactly that restatement — the view keeps the published
content alongside a queue of not-yet-raised changes, and applies each change at the moment its
notification is raised. Because the order is preserved, such a restatement exists.

Reentrancy admits no equivalent. A consumer that receives Reset and then Add 4@3 cannot
recover the truth from what it was given, because the two notifications arrived in the reverse of
the order in which the changes happened. There is no reference point against which those args are
valid. That is why it is the one to reject.

To be explicit: this proposal is about same-thread reentrancy only. Modification from another
thread is a supported scenario and must stay supported; it is already serialized by SyncRoot,
which is also what makes the guard cheap — at most one thread is dispatching a notification for a
given collection at any moment, so a plain depth field guarded by the same lock is enough, with no
thread identity to record.

Proposal

While a notification is being dispatched, reject any attempt to modify that collection from the
same thread.

There is direct precedent in the BCL, and its rationale is worded almost exactly like the problem
above — from ObservableCollection<T>.CheckReentrancy:

we can allow changes if there's only one listener - the problem only arises if reentrant
changes make the original event args invalid for later listeners. This keeps existing code
working (e.g. Selector.SelectedItems).

So .NET developers already know this constraint from ObservableCollection<T>, and the idiomatic
workaround — queue the change and apply it after the notification completes, or post it to a
dispatcher — is well established.

Worth noting that the single-listener exemption in that quote would buy almost nothing here: as
soon as CreateView or ToNotifyCollectionChanged is used, the view is itself a listener, so
there is virtually always more than one. Adopting WPF's exact rule would be indistinguishable
from rejecting unconditionally.

Why not defer the reentrant modification instead

Queueing the reentrant call and draining it after the notification completes also preserves the
order, and it would make #120's scenario work. But it cannot be generalized:

• Members that return a value cannot be deferred. Pop(), Dequeue(), RemoveFirst(),
  RemoveLast(), Remove(T), TryPop(out T) and friends would have to return a value that is
  not determined yet. ObservableStack.PopRange(Span<T> dest) is outright impossible: it has to
  fill a caller-supplied span, and a Span<T> cannot even be stored for later. These members
  would have to throw anyway, leaving a split API — void members defer, value-returning members
  throw — which is harder to explain than rejecting uniformly.
• It turns a crash into a hang. Two handlers that push each other (one removes when the count
  is too high, one adds when it is too low) currently blow the stack, which at least stops.
  Draining a queue instead loops forever at constant stack depth inside a single Add() call
  that never returns.
• It changes the timing of existing code silently. Rejection breaks reentrant callers at the
  exact call site; deferral keeps them compiling and running while quietly moving when their
  state changes.

Deferral is the right answer, but as something the caller opts into, not as an implicit library
behavior.

Relation to #120 / #121

Prohibition does not satisfy #120 — it changes the symptom rather than fixing it. In that
repro the observer's Clear() would be rejected, so events stays [1] and Count stays 1.
The reported count is still not [1, 0]; the only difference is that the failure is reported
instead of the count being dropped silently. (Where exactly it surfaces is question 3 below.)

So I think the right order is:

1. Merge #121 and close #120 on its own terms. The countPrev move is correct and, absent
   reentrancy, behaviourally identical to today's code, so it costs nothing to keep. It now
   covers both the IObservableCollection<T> and the ISynchronizedView<T, TView> overloads.
2. Introduce the reentrancy guard afterwards. At that point the two tests #121 adds —
   ObserveCountChanged_WithSideEffect and ObserveViewCountChanged_WithSideEffect — have to be
   replaced: they assert events == [1, 0] and Count == 0, which becomes "the side effect is
   rejected".

Open design questions

1. Members that have already mutated before they can report the rejection. If a different
   handler is the one that violates the guard, Pop() / Dequeue() / RemoveFirst() /
   RemoveLast() have already removed the element and cannot both throw and return it, so the
   element is lost. The guard probably has to be checked before mutating, not at notification
   time.
2. Writable views. IWritableSynchronizedView<T, TView> updates the view first and writes
   back to the source via SetToSourceCollection. If the write-back is rejected, the view and the
   source diverge.
3. How the rejection is reported. To the handler that attempted the modification, to the
   caller of the outermost mutating method, or both? And do the remaining handlers still receive
   the original notification?

Start with ObservableList mutation methods and CollectionChanged dispatch under SyncRoot, then inspect ObservableCollection.CheckReentrancy for the precedent described here. Review #120 and #121, including ObserveCountChanged_WithSideEffect and ObserveViewCountChanged_WithSideEffect; the work is not complete until the guard's timing, writable-view behavior, exception reporting, and remaining-handler behavior are decided and covered by tests.