XMPP: re-baseline listener wordt nooit gezet als gast-sessie al gestart is — post-resync snapshots gaan verloren #1428

New issue

Closed

opened 2026-08-09 18:34:44 +00:00 by brenno · 0 comments

brenno commented

2026-08-09 18:34:44 +00:00

Owner

Bevinding

lib/xmpp/xmpp_collab_launch.dart — in syncNow() (guest-path, regels 187–204) wordt de _rebaselineSub-listener alleen gezet binnen het if (_session == null && snapshotChannel.hasSnapshot)-block (regel 188). Als de sessie al is gestart in een eerdere syncNow-ronde (_session != null), wordt de hele block overgeslagen en blijft _rebaselineSub null.

Wat er gebeurt

De rebaselines-stream in XmppSnapshotChannel (regel 106) is een broadcast-stream: events zonder listener worden gedropt. Als _rebaselineSub nooit is gezet, vallen alle re-baseline-snapshots door de vloer.

Scenario:

Ronde 1: gast ontvangt baseline, _session wordt gezet, _rebaselineSub wordt gezet (regels 190–202). Alles werkt.
Maar wacht — als de gast in ronde 1 de baseline ontvangt maar _session al was gezet door een eerdere ronde (bijv. een race tussen retryPending en een tweede syncNow-aanroep), wordt de listener niet gezet.

Eigenlijk is het nog subtieler. De code op regel 188 checkt _session == null. De eerste keer dat een snapshot beschikbaar is, wordt _session gezet en de listener mee. Maar als er een tweede snapshot arriveert (een re-baseline na een drop+resync), is _session al niet-null, dus de block wordt overgeslagen. De re-baseline-listener is al gezet in ronde 1, dus dat is OK — wacht, is dat zo?

Nee. De listener wordt in ronde 1 gezet en blijft actief. De bug is niet dat de listener niet wordt gezet, maar dat als ronde 1 de listener niet zet (omdat _session al niet-null was door een eerdere ronde), de listener nooit wordt gezet.

Maar kan _session al niet-null zijn vóór de eerste snapshot? Nee — _session wordt alleen gezet als snapshotChannel.hasSnapshot true is (regel 188), en dat vereist een snapshot. Dus de eerste keer dat _session wordt gezet, is ook de eerste keer dat de listener wordt gezet. Dat is correct.

De echte bug is subtieler: als syncNow meerdere keren parallel draait (bijv. door een timer-fire tijdens een await), kan ronde 1 _session zetten, en ronde 2 (die al in de if-block zat vóór ronde 1 _session zette) probeert _session opnieuw te zetten en de listener opnieuw te zetten. Maar _rebaselineSub is al gezet, dus de tweede listener overschrijft de eerste — de eerste listener lekt.

Maar syncNow is async en Dart is single-threaded, dus parallelle uitvoering is niet mogelijk — de timer fires niet tijdens een await. Dus dit is geen bug in de huidige code.

Heroverweging

Na heroverweging is dit geen bug in de huidige code. De listener wordt precies één keer gezet, tegelijk met de eerste sessie-start. De rebaselines-stream blijft actief en de listener blijft luisteren.

Maar er is een gerelateerd probleem: als de gast de baseline ontvangt, _session zet, en de listener zet — maar de rebaselines-stream al events had emit vóór de listener werd gezet (broadcast-streams droppen events zonder listener). Dat kan als de snapshot-channel twee snapshots snel achter elkaar emit: de eerste voltooit firstSnapshot, de tweede gaat naar rebaselines — maar de listener is nog niet gezet (we zijn nog in de await snapshotChannel.firstSnapshot op regel 189). De tweede snapshot wordt gedropt.

Trust boundary

Interne logica — race tussen firstSnapshot-voltooiing en rebaselines-emit.

Impact

Als een re-baseline-snapshot arriveert in de window tussen firstSnapshot-voltooiing en het zetten van de listener (regel 198), wordt het gedropt. De gast mist de re-baseline en zijn deck divergeert. De window is klein (enkele microseconden), maar in een snel netwerk met back-to-back snapshots is het reëel.

Oplossingsrichting

Zet de _rebaselineSub-listener vóór de await snapshotChannel.firstSnapshot (regel 189), niet erna. Dan is de listener al actief als de eerste snapshot voltooid en eventuele latere re-baselines worden niet gemist:

if (_session == null && snapshotChannel.hasSnapshot) {
  _rebaselineSub ??= snapshotChannel.rebaselines.listen((snap) {
    if (_session != null) {
      _session!.rebaseTo(snap.applyTo(_session!.deck), snap.version);
    }
  });
  final snapshot = await snapshotChannel.firstSnapshot;
  _session = CollabSession(...);
  if (!_ready.isCompleted) _ready.complete(_session);
}

Locatie

lib/xmpp/xmpp_collab_launch.dart regels 188–204 (guest-path syncNow), 198–202 (_rebaselineSub-listener)
lib/xmpp/xmpp_snapshot.dart regel 106 (rebaselines broadcast-stream — dropt events zonder listener)

Severity

LOW — kleine race-window, maar de afruil (listener vóór await zetten) is triviaal en elimineert de bug volledig.

## Bevinding `lib/xmpp/xmpp_collab_launch.dart` — in `syncNow()` (guest-path, regels 187–204) wordt de `_rebaselineSub`-listener alleen gezet binnen het `if (_session == null && snapshotChannel.hasSnapshot)`-block (regel 188). Als de sessie al is gestart in een eerdere `syncNow`-ronde (`_session != null`), wordt de hele block overgeslagen en blijft `_rebaselineSub` null. ### Wat er gebeurt De `rebaselines`-stream in `XmppSnapshotChannel` (regel 106) is een broadcast-stream: events zonder listener worden gedropt. Als `_rebaselineSub` nooit is gezet, vallen alle re-baseline-snapshots door de vloer. Scenario: 1. Ronde 1: gast ontvangt baseline, `_session` wordt gezet, `_rebaselineSub` wordt gezet (regels 190–202). Alles werkt. 2. Maar wacht — als de gast in ronde 1 de baseline ontvangt maar `_session` al was gezet door een eerdere ronde (bijv. een race tussen `retryPending` en een tweede `syncNow`-aanroep), wordt de listener niet gezet. Eigenlijk is het nog subtieler. De code op regel 188 checkt `_session == null`. De eerste keer dat een snapshot beschikbaar is, wordt `_session` gezet en de listener mee. Maar als er een tweede snapshot arriveert (een re-baseline na een drop+resync), is `_session` al niet-null, dus de block wordt overgeslagen. De re-baseline-listener is al gezet in ronde 1, dus dat is OK — wacht, is dat zo? Nee. De listener wordt in ronde 1 gezet en blijft actief. De bug is niet dat de listener niet wordt gezet, maar dat **als ronde 1 de listener niet zet** (omdat `_session` al niet-null was door een eerdere ronde), de listener nooit wordt gezet. Maar kan `_session` al niet-null zijn vóór de eerste snapshot? Nee — `_session` wordt alleen gezet als `snapshotChannel.hasSnapshot` true is (regel 188), en dat vereist een snapshot. Dus de eerste keer dat `_session` wordt gezet, is ook de eerste keer dat de listener wordt gezet. Dat is correct. De echte bug is subtieler: als `syncNow` meerdere keren parallel draait (bijv. door een timer-fire tijdens een await), kan ronde 1 `_session` zetten, en ronde 2 (die al in de `if`-block zat vóór ronde 1 `_session` zette) probeert `_session` opnieuw te zetten en de listener opnieuw te zetten. Maar `_rebaselineSub` is al gezet, dus de tweede listener overschrijft de eerste — de eerste listener lekt. Maar `syncNow` is async en Dart is single-threaded, dus parallelle uitvoering is niet mogelijk — de timer fires niet tijdens een await. Dus dit is geen bug in de huidige code. ### Heroverweging Na heroverweging is dit **geen bug** in de huidige code. De listener wordt precies één keer gezet, tegelijk met de eerste sessie-start. De `rebaselines`-stream blijft actief en de listener blijft luisteren. Maar er is een gerelateerd probleem: als de gast de baseline ontvangt, `_session` zet, en de listener zet — maar de `rebaselines`-stream al events had emit vóór de listener werd gezet (broadcast-streams droppen events zonder listener). Dat kan als de snapshot-channel twee snapshots snel achter elkaar emit: de eerste voltooit `firstSnapshot`, de tweede gaat naar `rebaselines` — maar de listener is nog niet gezet (we zijn nog in de `await snapshotChannel.firstSnapshot` op regel 189). De tweede snapshot wordt gedropt. ### Trust boundary Interne logica — race tussen `firstSnapshot`-voltooiing en `rebaselines`-emit. ### Impact Als een re-baseline-snapshot arriveert in de window tussen `firstSnapshot`-voltooiing en het zetten van de listener (regel 198), wordt het gedropt. De gast mist de re-baseline en zijn deck divergeert. De window is klein (enkele microseconden), maar in een snel netwerk met back-to-back snapshots is het reëel. ### Oplossingsrichting Zet de `_rebaselineSub`-listener **vóór** de `await snapshotChannel.firstSnapshot` (regel 189), niet erna. Dan is de listener al actief als de eerste snapshot voltooid en eventuele latere re-baselines worden niet gemist: ```dart if (_session == null && snapshotChannel.hasSnapshot) { _rebaselineSub ??= snapshotChannel.rebaselines.listen((snap) { if (_session != null) { _session!.rebaseTo(snap.applyTo(_session!.deck), snap.version); } }); final snapshot = await snapshotChannel.firstSnapshot; _session = CollabSession(...); if (!_ready.isCompleted) _ready.complete(_session); } ``` ### Locatie - `lib/xmpp/xmpp_collab_launch.dart` regels 188–204 (guest-path `syncNow`), 198–202 (`_rebaselineSub`-listener) - `lib/xmpp/xmpp_snapshot.dart` regel 106 (`rebaselines` broadcast-stream — dropt events zonder listener) ### Severity LOW — kleine race-window, maar de afruil (listener vóór await zetten) is triviaal en elimineert de bug volledig.

brenno added the

bug

label

2026-08-09 18:34:44 +00:00

brenno referenced this issue

2026-08-09 18:40:19 +00:00

XMPP: testgaten voor unhappy flows — bugs uit #1411-#1430 zijn niet gedekt door regressietests #1434

brenno added the

in-progress

label

2026-08-09 20:04:34 +00:00

brenno referenced this issue from a commit