Fixed that `.Edit()` operations would not defer notifications for new subscriptions occurring during the edit. by JakenVeina · Pull Request #1133 · reactivemarbles/DynamicData

JakenVeina · 2026-07-04T07:09:12Z

Fixed that subscriptions occurring during an .Edit() operation upon a SourceCache<> or SourceList<> would still publish an initial notification to the subscriber. This resulted in the potential for notifications to be duplicated, for items added or manipulated during the edit.

Instead, such subscriptions now have their notifications deferred until the .Edit() is complete.

Resolves #1129.

dwcullop · 2026-07-25T22:50:48Z

+                    : Observable.CombineLatest(
+                            _suspensionTracker.Value.NotificationsSuspendedObservable,
+                            _isEditInProgress,
+                            static (areNotificationsSuspended, isEditInProgress) => areNotificationsSuspended || isEditInProgress)
+                        .Do(static _ => { }, observer.OnCompleted)
+                        .Where(static shouldConnectionBeDeferred => !shouldConnectionBeDeferred)
+                        .Take(1)
+                        .Select(_ => CreateConnectObservable(predicate, suppressEmptyChangeSets))
+                        .Switch();


A subscriber that connects while an edit is in progress never receives OnCompleted when the source is later disposed. It activates fine and receives every changeset, it just never terminates, so anything using Finally for cleanup or awaiting completion waits forever. SourceList behaves the same way.

This isn't yours: the suspension deferral on main already loses completion the same way, and this reuses that shape. The difference is the trigger. Connecting during a suspension is unusual, connecting during an edit is not, so the PR takes a corner case and makes it routine.

Whatever the fix is, it probably belongs on both paths at once, and it might be worth splitting out separately rather than growing this PR.

Also filed #1140. The same deferral converts a source failure into a successful completion, not just a lost completion: a deferred subscriber gets OnCompleted where an ordinary one gets OnError.

Relevant to this PR specifically: routing _isEditInProgress.OnError through the same Do(..., observer.OnCompleted) means an edit-deferred subscriber inherits that behaviour, which widens the trigger from suspensions to ordinary edits.

Let's take that one first then, and I'll make sure it gets properly incorporated here, when I update the branch.

dwcullop · 2026-07-25T22:51:27Z

Filed this as #1136 so it doesn't get lost if this PR lands first.

dwcullop

Thanks for working through all of that, it's in much better shape. The lazy seeding with _editLevel is not 0 is a neat touch, it keeps the mid-edit case correct without paying for the subject up front. Nested edits and the failed-edit recovery both have coverage now too.

Two things I'd like to sort out first, both in SourceList. Details inline.

One heads-up as well: this branch predates #1141, which reworked the same deferral in Connect/Watch. Merging main in will conflict there, so worth a careful resolution.

dwcullop

Went back over the four from last time against the current head, now that the merge has brought in #1137, #1139 and #1141.

Three of them are gone. Two were mine to withdraw: the list Switch one is fixed by #1139 landing, and the deferral one I simply had wrong, which I have owned in the thread. The benchmark question you had already answered.

The re-entrancy one in SourceList.Edit is still live and I think it is the last thing standing. A subscriber that edits the list from a change handler throws, because InvokeNext now runs while _editLevel is still 1 and the nested edit takes the WriteNested path after the outer write has finished. It works on main, so it is coming from this branch. Suggested shape is in the thread, and the suite is green with it.

Everything else from the first pass looks good. The lazy seeding, the transition guards, SourceList getting the same treatment as the cache, and the extra coverage for nested edits and removals all read well.

… a `SourceCache<>` or `SourceList<>` would still publish an initial notification to the subscriber. This resulted in the potential for notifications to be duplicated, for items added or manipulated during the edit. Instead, such subscriptions now have their notifications deferred until the `.Edit()` is complete. Resolves #1129.

…cking doesn't fall apart if user-injected code throws.

….SuspendNotifications()` does not initialize unless necessary.

…m to the edit-tracking system as well. Also extended the proper strategy for lazy-initialization of both systems from `.Connect()` to `.Watch()`.

…ceList, with some context clarification instead.

…nd nested edits.

…stem in `ObservableCache` to the edit-tracking system in `SourceList`.

… subscriptions occurring during the edit. (#1133) * Fixed that subscriptions occurring during an `.Edit()` operation upon a `SourceCache<>` or `SourceList<>` would still publish an initial notification to the subscriber. This resulted in the potential for notifications to be duplicated, for items added or manipulated during the edit. Instead, such subscriptions now have their notifications deferred until the `.Edit()` is complete. Resolves #1129. * Added exception guarantees to edit level tracking, to ensure that tracking doesn't fall apart if user-injected code throws. * Adjusted `.Connect()` to ensure that the internal system to support `.SuspendNotifications()` does not initialize unless necessary. * Extended the strategy of lazy-initialization for the suspension system to the edit-tracking system as well. Also extended the proper strategy for lazy-initialization of both systems from `.Connect()` to `.Watch()`. * Added logic to avoid redundant `_isEditInProgress` notifications. * Removed redundant completion signal on _isEditInProgress, within SourceList, with some context clarification instead. * Adjusted the new test for covering #1129, to also exercise removals and nested edits. * Extended the strategy of lazy-initialization for the edit-tracking system in `ObservableCache` to the edit-tracking system in `SourceList`. --------- Co-authored-by: Darrin W. Cullop <Darrin.Cullop@microsoft.com>

JakenVeina requested review from RolandPheasant and dwcullop and removed request for dwcullop July 7, 2026 06:41

dwcullop mentioned this pull request Jul 25, 2026

Fixed #1131 - Race condition between SuspendNotifications() and .ResumeNotifications() #1132

Merged