Skip to content

takeUntil: no ordering guarantee between source values and the notifier #34

Description

@dbrattli

Split out of #33, which fixed the test that was tripping over this. The operator behaviour itself is untouched and worth deciding on deliberately.

What happens

takeUntil subscribes the notifier and the source independently, and both deliver into the same safeObserver:

let! sub2 = AsyncObserver _obv |> other.SubscribeAsync   // notifier
let! sub1 = source.SubscribeAsync safeObv |> autoDetach  // source

Each side is several actor hops deep and the two chains drain concurrently:

values:     xs mb actor → xs sobv actor → takeUntil safeObv actor → downstream
completion: ys mb actor → ys sobv actor ↗

So a value posted to the source before the notifier fires can still arrive at the shared observer after the notifier's OnCompleted. safeObserver has already set stopped by then:

if stopped then
    return! messageLoop stopped

and the value is silently dropped.

Why it matters

Wall-clock ordering at the producer does not survive to the consumer. A caller who does

do! source.OnNextAsync 1
do! source.OnNextAsync 2
do! notifier.OnNextAsync true

has no guarantee of seeing 1 and 2. It usually works — the source was posted first and has items already in flight — which makes the failure mode a rare, load-dependent silent data loss rather than an obvious error. This is exactly how it showed up: a single CI failure on #32 that passed on a re-run of the same commit, and 0/65 reproductions locally across 16-, 2- and 1-core runs.

The question

Rx implementations generally do not promise cross-source ordering here either — concurrency between two independent sequences is the user's problem, and takeUntil is inherently racy in Rx.NET and RxJS too. So there are three defensible positions:

  1. Document it. Add the caveat to the takeUntil docstring: values racing the notifier may be dropped; do not rely on producer-side ordering across two independent sources. Cheapest, matches Rx precedent.
  2. Serialize the two chains. Route the notifier through the same mailbox as the source so a single queue orders them. Gives the intuitive guarantee, at the cost of an extra hop on the value path, and would need care not to reintroduce the stale-child class of bug fixed in feat(beam): port src to the BEAM target and wire cross-target test runs #25.
  3. Leave as is. Accept it as an unstated Rx-ism.

I lean toward 1 — the behaviour matches the wider Rx ecosystem, and the real defect was a test asserting an interleaving it had not arranged. But it should be a written-down decision rather than an accident, since the failure is silent.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions