Skip to content

agent: fix three races that closing clients on a network change makes reachable - #1928

Closed
Narasimha-sc wants to merge 1 commit into
nd/net-close-clientsfrom
nd/net-session-races
Closed

Narasimha-sc wants to merge 1 commit into
nd/net-close-clientsfrom
nd/net-session-races

Conversation

@Narasimha-sc

@Narasimha-sc Narasimha-sc commented Oct 5, 2026 •

Copy link
Copy Markdown

Closing the clients on a network change makes a client that is no longer the one in the session map a routine occurrence rather than a rare one, which makes these pre-existing races reachable in normal use. None of them is introduced by this stack; all three would leave state wrong after a reconnect.

  • setSubsPending removes the subscriptions of a transport session only when the session id matches. A client that is no longer the one in the map never set its session id, and when the session mode stopped matching, its disconnection removed the subscriptions of the client that replaced it without making them pending - so they were neither subscribed nor scheduled, and nothing corrected it until the app restarted.
  • smpClientDisconnected removes the sessions to the proxied relays only when no other client replaced this one, as those sessions are then the sessions of the client that replaced it.
  • updateClientService reads the stored service id in the transaction that writes it. The id read before the handshake can be stale, as the transaction of a client that is no longer current can commit after the one of the client that replaced it, and removeRcvServiceAssocs would then run against a service id that changed since, removing the associations of the current client.

This PR must land after the client-closing change, not before it: setSubsPending's guard relies on newProtocolClient setting the session id only while its var is the one in the map.


Stack

Reducing reconnection delay after a network change. Each PR is based on the previous one and can be merged on its own once the ones below it are in.

PR base
#1926 transport: bound unacknowledged-data failure time master 894 s -> 95 s, no app change, independent
#1927 agent: reconnect to the servers when the network changes #1926 the 15-minute stall
#1928 agent: fix three races closing clients makes reachable #1927 correctness guard for #1927
#1929 agent: restart the retry loops when the network changes #1928 retry interval -> ~3 s
#1930 agent: restart retry loops when reconnecting all servers #1929 manual reconnect only
simplex-chat/simplex-chat#7650 android, ios: pass the chat controller to the network info command master crash fix, independent
simplex-chat/simplex-chat#7651 android, ios: report a network change when the network identity changes #7650 app side of simplexmq#1927
simplex-chat/simplex-chat#7652 desktop: report network changes to the core #7651 new capability

… reachable

Closing the clients on a network change makes a client that is no longer the one
in the session map a routine occurrence rather than a rare one, which makes these
pre-existing races reachable in normal use.

- setSubsPending removes the subscriptions of a transport session only when the
  session id matches. A client that is no longer the one in the map never set
  its session id, and when the session mode stopped matching the transport
  session its disconnection removed the subscriptions of the client that
  replaced it without making them pending, so they were neither subscribed nor
  scheduled and nothing corrected it until the app restarted.
- smpClientDisconnected removes the sessions to the proxied relays only when no
  other client replaced this one, as those sessions are then the sessions of the
  client that replaced it.
- updateClientService reads the stored service id in the transaction that writes
  it. The id read before the handshake can be stale, as the transaction of a
  client that is no longer current can commit after the one of the client that
  replaced it, and removeRcvServiceAssocs would then run against a service id
  that changed since, removing the associations of the current client.
This was referenced Oct 20, 2026
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.

1 participant