Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
942b762
List: replace .Synchronize(locker) with SDQ-based safe variants in 15…
dwcullop Jun 15, 2026
1bc49f6
List: migrate SourceList to DeliveryQueue and rewrite ExpireAfter shadow
dwcullop Jun 15, 2026
56b8705
Tests: add list cross-mutation deadlock torture suite
dwcullop Jun 15, 2026
8f9f2e6
Address adversarial review: fix Dispose race, ExpireAfter duplicates,…
dwcullop Jun 15, 2026
0c5920e
Merge branch 'main' into pr/list-deadlock-immunity
dwcullop Jun 15, 2026
ee80e29
SourceList: gate Preview on terminal-enqueued flag so it cannot fire …
dwcullop Jun 15, 2026
6557e49
SourceList: remove double-OnCompleted from Connect, wrap _editLevel i…
dwcullop Jun 15, 2026
8747b3a
TransformMany: use per-child DeliveryQueue instead of pointless per-c…
dwcullop Jun 15, 2026
0ec74be
TransformMany: one shared queue for the whole operator, each per-chil…
dwcullop Jun 15, 2026
847eeb3
Address multi-agent review (PR-A pass 2): fix ExpireAfter prefix-chec…
dwcullop Jun 15, 2026
56abf86
Remove stray review-agent test files
dwcullop Jun 15, 2026
1c784eb
List: replace post-queue Merge/CombineLatest with UnsynchronizedMerge…
dwcullop Jun 15, 2026
c9a9155
List: replace remaining gate-holding Rx combinators per #1097 audit
dwcullop Jun 15, 2026
30c1248
Merge branch 'reactivemarbles:main' into pr/list-deadlock-immunity
dwcullop Jun 20, 2026
badf28f
Merge branch 'main' into pr/list-deadlock-immunity
dwcullop Jul 31, 2026
b324f80
Merge branch 'main' into pr/list-deadlock-immunity
dwcullop Jul 31, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/DynamicData/List/Internal/BufferIf.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using System.Reactive.Linq;
using System.Reactive.Subjects;

using DynamicData.Internal;

namespace DynamicData.List.Internal;

internal sealed class BufferIf<T>(IObservable<IChangeSet<T>> source, IObservable<bool> pauseIfTrueSelector, bool initialPauseState = false, TimeSpan? timeOut = null, IScheduler? scheduler = null)
Expand All @@ -23,13 +25,16 @@ internal sealed class BufferIf<T>(IObservable<IChangeSet<T>> source, IObservable
public IObservable<IChangeSet<T>> Run() => Observable.Create<IChangeSet<T>>(
observer =>
{
var locker = InternalEx.NewLock();
// SharedDeliveryQueue + SynchronizeSafe replaces Synchronize(locker) so the
// gate lock is released before downstream OnNext. Closes the cross-cache
// deadlock window.
var queue = new SharedDeliveryQueue();
var paused = initialPauseState;
var buffer = new ChangeSet<T>();
var timeoutSubscriber = new SerialDisposable();
var timeoutSubject = new Subject<bool>();

var bufferSelector = Observable.Return(initialPauseState).Concat(_pauseIfTrueSelector.Merge(timeoutSubject)).ObserveOn(_scheduler).Synchronize(locker).Publish();
var bufferSelector = Observable.Return(initialPauseState).Concat(_pauseIfTrueSelector.Merge(timeoutSubject)).ObserveOn(_scheduler).SynchronizeSafe(queue).Publish();

var pause = bufferSelector.Where(state => state).Subscribe(
_ =>
Expand Down Expand Up @@ -61,7 +66,7 @@ public IObservable<IChangeSet<T>> Run() => Observable.Create<IChangeSet<T>>(
timeoutSubscriber.Disposable = Disposable.Empty;
});

var updateSubscriber = _source.Synchronize(locker).Subscribe(
var updateSubscriber = _source.SynchronizeSafe(queue).Subscribe(
updates =>
{
if (paused)
Expand All @@ -85,6 +90,7 @@ public IObservable<IChangeSet<T>> Run() => Observable.Create<IChangeSet<T>>(
updateSubscriber.Dispose();
timeoutSubject.OnCompleted();
timeoutSubscriber.Dispose();
queue.Dispose();
});
});
}
46 changes: 21 additions & 25 deletions src/DynamicData/List/Internal/Combiner.cs
Original file line number Diff line number Diff line change
@@ -1,51 +1,47 @@
// Copyright (c) 2011-2025 Roland Pheasant. All rights reserved.
// Copyright (c) 2011-2025 Roland Pheasant. All rights reserved.
// Roland Pheasant licenses this file to you under the MIT license.
// See the LICENSE file in the project root for full license information.

using System.Reactive.Disposables;
using System.Reactive.Linq;

using DynamicData.Cache.Internal;
using DynamicData.Internal;

namespace DynamicData.List.Internal;

internal sealed class Combiner<T>(ICollection<IObservable<IChangeSet<T>>> source, CombineOperator type)
where T : notnull
{
#if NET9_0_OR_GREATER
private readonly Lock _locker = new();
#else
private readonly object _locker = new();
#endif

private readonly ICollection<IObservable<IChangeSet<T>>> _source = source ?? throw new ArgumentNullException(nameof(source));

public IObservable<IChangeSet<T>> Run() => Observable.Create<IChangeSet<T>>(
observer =>
{
var disposable = new CompositeDisposable();

var resultList = new ChangeAwareListWithRefCounts<T>();
var sourceLists = Enumerable.Range(0, _source.Count).Select(_ => new ReferenceCountTracker<T>()).ToList();

lock (_locker)
{
var sourceLists = Enumerable.Range(0, _source.Count).Select(_ => new ReferenceCountTracker<T>()).ToList();
// Shared queue serializes the multiple source streams so they appear as a
// single sequence to the combiner, but without holding a lock during the
// downstream observer.OnNext call.
var queue = new SharedDeliveryQueue();

foreach (var pair in _source.Zip(sourceLists, (item, list) => new { Item = item, List = list }))
{
disposable.Add(
pair.Item.Synchronize(_locker).Subscribe(
changes =>
foreach (var pair in _source.Zip(sourceLists, (item, list) => new { Item = item, List = list }))
{
disposable.Add(
pair.Item.SynchronizeSafe(queue).Subscribe(
changes =>
{
CloneSourceList(pair.List, changes);

var notifications = UpdateResultList(changes, sourceLists, resultList);
if (notifications.Count != 0)
{
CloneSourceList(pair.List, changes);

var notifications = UpdateResultList(changes, sourceLists, resultList);
if (notifications.Count != 0)
{
observer.OnNext(notifications);
}
}));
}
observer.OnNext(notifications);
}
},
observer.OnError));
}

return disposable;
Comment thread
dwcullop marked this conversation as resolved.
Expand Down
14 changes: 7 additions & 7 deletions src/DynamicData/List/Internal/DisposeMany.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
using System.Reactive.Disposables;
using System.Reactive.Linq;

using DynamicData.Internal;

namespace DynamicData.List.Internal;

internal sealed class DisposeMany<T>(IObservable<IChangeSet<T>> source)
Expand All @@ -14,11 +16,13 @@ internal sealed class DisposeMany<T>(IObservable<IChangeSet<T>> source)
public IObservable<IChangeSet<T>> Run()
=> Observable.Create<IChangeSet<T>>(observer =>
{
// Will be locking on cachedItems directly, instead of using an anonymous gate object. This is acceptable, since it's a privately-held object, there's no risk of deadlock from other consumers locking on it.
var cachedItems = new List<T>();

// SynchronizeSafe with queue-first disposal: the DeliveryQueue is terminated and
// drained before the source subscription is disposed, ensuring no notifications
// can fire while the teardown (per-item Dispose) is in progress.
var sourceSubscription = source
.Synchronize(cachedItems)
.SynchronizeSafe()
.SubscribeSafe(Observer.Create<IChangeSet<T>>(
onNext: changeSet =>
{
Expand Down Expand Up @@ -76,11 +80,7 @@ public IObservable<IChangeSet<T>> Run()
return Disposable.Create(() =>
{
sourceSubscription.Dispose();

lock (cachedItems)
{
ProcessFinalization(cachedItems);
}
ProcessFinalization(cachedItems);
});
});

Expand Down
18 changes: 9 additions & 9 deletions src/DynamicData/List/Internal/DynamicCombiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,32 @@
using System.Reactive.Linq;

using DynamicData.Cache.Internal;
using DynamicData.Internal;

namespace DynamicData.List.Internal;

internal sealed class DynamicCombiner<T>(IObservableList<IObservable<IChangeSet<T>>> source, CombineOperator type)
where T : notnull
{
#if NET9_0_OR_GREATER
private readonly Lock _locker = new();
#else
private readonly object _locker = new();
#endif

private readonly IObservableList<IObservable<IChangeSet<T>>> _source = source ?? throw new ArgumentNullException(nameof(source));

public IObservable<IChangeSet<T>> Run() => Observable.Create<IChangeSet<T>>(
observer =>
{
// SharedDeliveryQueue + SynchronizeSafe replaces Synchronize(_locker) so the
// gate is released before downstream OnNext. Closes the cross-cache deadlock
// window.
var queue = new SharedDeliveryQueue();

// this is the resulting list which produces all notifications
var resultList = new ChangeAwareListWithRefCounts<T>();

// Transform to a merge container.
// This populates a RefTracker when the original source is subscribed to
var sourceLists = _source.Connect().Synchronize(_locker).Transform(changeSet => new MergeContainer(changeSet)).AsObservableList();
var sourceLists = _source.Connect().SynchronizeSafe(queue).Transform(changeSet => new MergeContainer(changeSet)).AsObservableList();

// merge the items back together
var allChanges = sourceLists.Connect().MergeMany(mc => mc.Source).Synchronize(_locker).Subscribe(
var allChanges = sourceLists.Connect().MergeMany(mc => mc.Source).SynchronizeSafe(queue).Subscribe(
changes =>
{
// Populate result list and check for changes
Expand Down Expand Up @@ -86,7 +86,7 @@ public IObservable<IChangeSet<T>> Run() => Observable.Create<IChangeSet<T>>(
}
}).Subscribe();

return new CompositeDisposable(sourceLists, allChanges, removedItem, sourceChanged);
return new CompositeDisposable(sourceLists, allChanges, removedItem, sourceChanged, queue);
});

private bool MatchesConstraint(MergeContainer[] sourceLists, T item)
Expand Down
Loading
Loading