Description

ObservablePropertyFactory (in src/DynamicData/Binding/ObservablePropertyFactory.cs) has a TOCTOU race that causes PropertyChanged events fired during the Subscribe call to be silently lost. Both the shallow form (single property) and the deep-chain form (Parent.Child.Property) are affected.

The bug manifests in any code that observes properties of objects mutated from multiple threads (background tasks updating ViewModels, services pushing into observable models, etc.). When a mutation lands in the subscribe-time race window, downstream observers never see that change.

Root cause

Both constructors in ObservablePropertyFactory<TObject, TProperty> use the pattern initial.Concat(events):

// Shallow form
var initial = Observable.Defer(() => Observable.Return(Factory()));
return initial.Concat(propertyChanged);

Concat subscribes to the second source only after the first completes. So on subscribe:

1. Defer evaluates Factory() which reads the property — TIME OF CHECK (snapshot V0)
2. Return(V0) emits → OnCompleted
3. Concat now subscribes to propertyChanged → FromEventPattern attaches the handler — TIME OF USE

Between steps 1 and 3 (a synchronous gap of hundreds of nanoseconds), any thread that mutates the property fires PropertyChanged with no subscriber attached. The mutation is silently lost.

The deep-chain form has the same race PLUS an additional gap: Take(1).Repeat tears down all chain notifiers and re-subscribes via GetNotifiers on every notifier fire, losing any events that arrive during the re-walk.

Reproduction

The race is small in normal code but can be forced deterministically by parking the observer's OnNext for the initial value on one thread while mutating from another:

[Fact]
public void Shallow_ConcurrentMutationDuringInitialEmit_NotDropped()
{
    var model = new TestModel { Value = 10 };
    var emissions = new List<int>();
    var observerInitialReceived = new ManualResetEventSlim(false);
    var observerCanContinue = new ManualResetEventSlim(false);

    var observer = Observer.Create<PropertyValue<TestModel, int>>(pv =>
    {
        bool isFirst;
        lock (emissions) { isFirst = emissions.Count == 0; emissions.Add(pv.Value); }
        if (isFirst) { observerInitialReceived.Set(); observerCanContinue.Wait(); }
    });

    var subscribeTask = Task.Run(() =>
        model.WhenPropertyChanged(m => m.Value, notifyOnInitialValue: true).Subscribe(observer));

    observerInitialReceived.Wait();
    model.Value = 20;          // mutation while Subscribe is parked in observer.OnNext
    observerCanContinue.Set();
    subscribeTask.Wait();
    Thread.Sleep(100);

    emissions.Should().Contain(20);   // FAILS on current code; mutation lost
}

private sealed class TestModel : INotifyPropertyChanged
{
    private int _value;
    public event PropertyChangedEventHandler? PropertyChanged;
    public int Value
    {
        get => _value;
        set { _value = value; PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Value))); }
    }
}

On main, emissions is {10} only. The mutation to 20 fires PropertyChanged with no subscriber attached (because Concat hasn't moved to subscribe to propertyChanged yet — it's blocked inside observer.OnNext(10)).

In real multi-threaded usage, the race window is small but nonzero. Symptoms: the UI didn't update reports that nobody can reproduce because the race is timing-dependent on the scheduler.

Fix outline

Subscribe to PropertyChanged BEFORE reading the initial value. Use Interlocked.CompareExchange for first emission wins so the initial and the first handler-fired event cannot both reach the observer. A one-shot equality guard at the boundary catches the rare setter-update-then-notify duplicate that the CAS cannot otherwise distinguish.

For the deep-chain form, replace Take(1).Repeat with per-level SerialDisposable slots. When a level's notifier fires, atomically swap deeper levels' subscriptions to point at the new value via SerialDisposable.Disposable = newSub (subscribes new, then disposes old). Every live chain level always has an active notifier; no re-walk gap.

Both fixes are lock-free (Interlocked + Volatile only).

Affected versions

All versions through current main. The pattern has been present since the original implementation.