Skip to content

proxy client: resubscribe on push for another session#881

Open
cyrilleberaud wants to merge 1 commit into
savoirfairelinux:masterfrom
cyrilleberaud:fix-push-session-resubscribe
Open

proxy client: resubscribe on push for another session#881
cyrilleberaud wants to merge 1 commit into
savoirfairelinux:masterfrom
cyrilleberaud:fix-push-session-resubscribe

Conversation

@cyrilleberaud

Copy link
Copy Markdown
Contributor

Problem

On iOS, the main app and the notification service extension (NSE) each run their own DhtProxyClient sharing the same push token and client_id, but each instance generates a random pushSessionId_. Whenever one instance subscribes a key on the proxy, the proxy re-associates the push subscription with that instance's session id.

Typical failure scenario (observed with Jami on a locked iPad receiving a call):

  1. A message push wakes the NSE, which runs a short-lived daemon; its proxy client subscribes with a new random session id. The proxy now tags all pushes with the NSE's session id.
  2. An incoming call arrives; CallKit wakes the main app.
  3. All queued push notifications fed to pushNotificationReceived() are rejected with IgnoredWrongSession (s= doesn't match the app's session), losing the push → immediate get() fast path.
  4. The app only discovers the pending PeerConnectionRequest after an unrelated internal resubscription — measured at ~3.5 s later.

Consequences for calls: the caller stays in "searching" state for several extra seconds while the callee already rings, and answering too early fails because the ICE answer hasn't been sent yet.

Measured timeline (Mac → locked iPad, before / after this patch on the callee):

Step Before After (expected)
Pushes accepted at app wake-up all rejected (128) resubscribe → values
PeerConnectionRequest delivered to app +3.4 s after wake ~0.3 s after wake
Caller sees RINGING +8.7 s ~+5 s

Fix

A push tagged with another session still carries a valid piece of information: values are available for this key. On session mismatch, if the key is one we currently listen on, resubscribe the listeners (with refresh=true, which both retrieves pending values immediately and re-associates the proxy-side subscription with this session) instead of dropping the notification, and return ListenRefresh.

Pushes for keys we do not listen on (e.g. leftovers addressed to a previous install) are still ignored with IgnoredWrongSession as before.

This is bounded and loop-free: a resubscription only happens when an actual push is delivered for a listened key, and APNs throttles push delivery. In the common single-instance case the behavior is unchanged.

On iOS, the app and the notification service extension each run their
own DhtProxyClient sharing the same push token and client_id, but with
different random session ids. Whenever one of them subscribes a key,
the proxy re-associates the push subscription with that instance's
session id.

As a result, when the extension has been running while the app was
suspended (e.g. to handle a message notification), all subsequent
pushes are tagged with the extension's session id. Once the app is
woken up (for instance by CallKit for an incoming call), every queued
push notification fed to pushNotificationReceived() is rejected with
IgnoredWrongSession, losing the push -> get() fast path. The app then
has to wait for an unrelated internal resubscription to discover
pending values: in practice this adds ~3.5 seconds before an incoming
call's PeerConnectionRequest is answered, keeping the caller in a
"searching" state and widening the window in which answering the call
too early makes it fail.

A push notification tagged with another session still carries a valid
piece of information: values are available for the given key. So, on
session mismatch, if the key is one we currently listen on, resubscribe
the listeners (with refresh=true, which both retrieves pending values
immediately and re-associates the proxy-side subscription with this
session) instead of dropping the notification. Pushes for keys we do
not listen on (e.g. leftovers for a previous install) are still
ignored as before.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates DhtProxyClient push-notification handling to better support multi-process iOS setups (main app + Notification Service Extension) that share the same push token/client ID but use different pushSessionId_ values. On session mismatch, it now attempts to resubscribe listeners (with refresh) for keys the client is actively listening on, instead of always ignoring the push.

Changes:

  • On push session mismatch, detect whether the push is for a key currently listened to by this client instance.
  • If listened, resubscribe listeners (refresh) to immediately retrieve pending values and re-associate the proxy-side subscription with this session.
  • Preserve the existing behavior for non-listened keys by continuing to return IgnoredWrongSession.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/dht_proxy_client.cpp
Comment on lines +1273 to +1283
std::lock_guard lock(searchLock_);
auto search = searches_.find(InfoHash(keyIt->second));
if (search != searches_.end() and not search->second.listeners.empty()) {
if (logger_)
logger_->debug("[proxy:client] [push] wrong session for [search {}], resubscribing",
search->first);
for (auto& list : search->second.listeners)
resubscribe(search->first, list.first, list.second);
loopSignal_();
return PushNotificationResult::ListenRefresh;
}
Comment thread src/dht_proxy_client.cpp
Comment on lines +1263 to +1267
// The push was requested by another client instance sharing this
// push token and client id (e.g. the iOS notification extension
// subscribing while the app was suspended): the proxy now tags
// pushes with that other session. The notification still means
// that values are available for the given key, so if we listen
@aberaud
aberaud requested a review from katekm July 15, 2026 20:29
@katekm

katekm commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Tested in calls — no improvement on the call path.

Note: For the branch to fire, the app must have been suspended before the call.

The incoming call's value is delivered by the initial subscribe created when the account is reactivated as the app wakes.
In the tests where the branch did fire, the value had already been delivered earlier by that subscribe, so the branch does not deliver the call value.

pushSessionId_ is regenerated per DhtProxyClient, so incoming pushes often carry an old session id — but the initial subscribe (refresh=false) already pulls the values, so the rejected pushes are redundant.

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.

3 participants