Skip to content

fix: reuse the DDP subscription a stream already has (#372) - #373

Open
diegolmello wants to merge 11 commits into
mobilefrom
diegolmello/bug-subscribe-mints-a-new-id-for-an-already-subs
Open

fix: reuse the DDP subscription a stream already has (#372)#373
diegolmello wants to merge 11 commits into
mobilefrom
diegolmello/bug-subscribe-mints-a-new-id-for-an-already-subs

Conversation

@diegolmello

@diegolmello diegolmello commented Aug 19, 2026

Copy link
Copy Markdown
Member

Proposed changes

Closes #372.

Socket.subscribe(name, params) with no explicit id always minted a new DDP subscription id, even when an identical stream was already recorded. reopenNow() does not close, so records survive a reconnect, and any caller that re-subscribes in its own post-login flow landed a second record for the same stream — one more per reconnect, each costing a sub frame and a callback invocation per incoming event.

subscribe now hands back the DDP subscription a stream already has, whether recorded or still in flight. The explicit-id resume path (subscribeAll, resubscribeWhenRecorded) is unchanged.

Stream identity is exact, since the reuse turns on it. findSubscriptions matches params by prefix, which would collapse distinct stream-notify-logged streams into one, and compares elements by reference, which never matches the options object every Driver.subscribe rebuilds per call — the streams the issue measured growing. Identity is the serialized name and params in full, keys sorted.

Two callers sharing one DDP subscription means the first unsubscribe would end the stream for both, so holders are counted per id and the unsub waits for the last of them. A count is taken only when a caller is handed a subscription, so the entry an abandoned sub writes — which resolves undefined — has no holder. unsubscribeAll drops the counts, since a teardown of everything ends a stream whoever else holds it. An entry whose unsub is already on the wire is not reusable, or the new caller receives a stream the server is about to end with no sub ever sent.

ADR-0011 records the decision.

Steps to reproduce

  1. Connect and log in.
  2. Subscribe to a stream via subscribe(name, params) with no explicit id.
  3. Drop the connection so reopenNow() runs.
  4. Let it reconnect and log in again, with the caller re-running its post-login subscribe for the same stream.
  5. Inspect socket.subscriptions — before this change, two records for one stream; after it, one.
  6. Repeat from 3 — before this change the count grew by the same amount each cycle.

Tests

  • reuses the subscription rather than minting a second id
  • reuses it when the params are rebuilt, as every Driver subscribe rebuilds them
  • reads the same params written in a different order as one stream
  • keeps two streams of the same name apart
  • delivers the stream to the second subscriber too
  • collapses two subscribes in flight at once
  • does not grow the registry when a reconnect re-runs the caller's subscribe
  • subscribes afresh while an unsubscribe is still on the wire
  • subscribes afresh once the subscriptions are forgotten, rather than reusing the request
  • subscribes afresh once every subscription is unsubscribed, rather than reusing the request
  • counts no holder for a subscriber the reconnect left with nothing
  • ends a shared stream when unsubscribing from all, whoever else holds it
  • unsubscribes only once the last holder lets go
  • lets the next holder end a stream whose unsubscribe the SDK itself rejected
  • leaves the second caller with nothing when the server refuses the subscribe they joined

`subscribe(name, params)` with no explicit id always minted a new DDP
subscription id, even for a stream already recorded. `reopenNow()` does not
close, so the records survive a reconnect, and any caller that re-subscribes as
part of its own post-login flow landed a second record for the same stream.
Each reconnect added another: one `sub` frame and one callback invocation per
accumulated record.

The reuse turns on stream identity, so it has to be exact. `findSubscriptions`
matches params by prefix, which would collapse distinct `stream-notify-logged`
streams into one, and its element comparison is by reference, which never
matches the options object every `Driver.subscribe` rebuilds per call — the
streams the issue measured growing. Identity is now the serialized name and
params in full, with object keys sorted.

Two callers sharing one DDP subscription means the first `unsubscribe` would
end the stream for both, so holders are counted per id and the `unsub` waits
for the last of them. A count is taken only when a caller is handed a
subscription, so the entry an abandoned `sub` writes — which resolves
`undefined` — has no holder. `unsubscribeAll` drops the counts, since a
teardown of everything ends a stream whoever else holds it.

An entry whose `unsub` is already on the wire is not reusable: reusing it hands
the new caller a stream the server is about to end, with no `sub` ever sent.

See ADR-0011.
A subscribe arriving while unsubscribeAll ran awaited a stale request whose subscription was gone, and was answered with nothing.

Also names the holder rule positively as hasOtherHolders, patches one subscription state record instead of rebuilding it, and drops the annotating comments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] subscribe() mints a new id for an already-subscribed stream, so the registry grows on every reconnect

1 participant