Skip to content

fix(coordinators): close empty-pubkeys → re-add subscription race in LocationCoordinator #96

@variablefate

Description

@variablefate

Background

Surfaced during the PR #93 (issue #91) review. Pre-existing across all three managed subscriptions in LocationCoordinator — not introduced by #91, but worth closing for clarity and resilience. Filed as a separate issue per the "don't widen scope" constraint on #91.

The race

startLocationSubscriptions, startKeyShareSubscription, and startDriverAvailabilitySubscription all share this shape when pubkeys becomes empty:

```swift
let previous = takeXSubscription() // nils field, returns prior

guard !pubkeys.isEmpty else {
if let previous {
Task {
previous.task.cancel()
await relayManager.unsubscribe(previous.id) // <-- async, detached
}
}
return
}
```

Subscription IDs are stable strings ("roadflare-locations", "key-shares", "driver-availability"). After the empty-path call, the field is nil. If a second call comes in immediately with non-empty pubkeys (e.g. user removes their last driver, then immediately re-follows), the second call:

  1. Sees previous = nil (already consumed)
  2. Creates a NEW Task that calls relayManager.subscribe(filter:, id: subId) with the SAME string ID

Meanwhile, the first call's detached unsubscribe Task is still in flight. If its await relayManager.unsubscribe(prev.id) resolves AFTER the second call's subscribe(...) completes, the relay-side CLOSE silently tears down the new subscription. The next set of events for that filter never arrives at the app — until some unrelated trigger restarts the subscription.

Why it hasn't been observed (probably)

  • Empty → non-empty transitions in user followed-driver lists are rare in production.
  • Most relays process REQ/CLOSE in the order received over a single connection, so the race window is small.
  • Lost events are silent: the app doesn't surface "I expected an event and didn't get one."

Suggested fix (uniform across all three subscriptions)

Switch to per-subscription unique IDs by appending the generation UUID:

```swift
let subId = SubscriptionID("driver-availability-(generation.uuidString)")
```

The empty-path detached unsubscribe targets the OLD subscription's unique ID, which is never reused, so it cannot affect any future subscription. The relay manager's internal map keys by the unique ID, so each REQ/CLOSE is isolated.

Apply identically in startLocationSubscriptions, startKeyShareSubscription, and startDriverAvailabilitySubscription. No callers depend on the string format; the IDs are only round-tripped through ManagedSubscription.

Why this wasn't fixed in PR #93

PR #93 mirrors the existing pattern (commit 81c91a7 introduced the location/key-share variants of this code). Fixing only the new third subscription would create inconsistency; fixing all three is a coordinator-wide subscription identity refactor unrelated to issue #91's vehicle scope. Filed here so it lands cleanly across all three at once.

Effort

Small — one-line change in three functions plus a quick test pinning that SubscriptionID no longer collides across calls. Could pair with a regression test that empties → repopulates pubkeys quickly and asserts events still flow.

Files

  • RoadFlare/RoadFlareCore/ViewModels/LocationCoordinator.swift

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions