Skip to content

SnodePool: fix use-after-free when running post-refresh callbacks - #105

Merged
mpretty-cyro merged 4 commits into
session-foundation:devfrom
mpretty-cyro:fix/snode-pool-crash
Jul 30, 2026
Merged

SnodePool: fix use-after-free when running post-refresh callbacks#105
mpretty-cyro merged 4 commits into
session-foundation:devfrom
mpretty-cyro:fix/snode-pool-crash

Conversation

@mpretty-cyro

@mpretty-cyro mpretty-cyro commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Fixes a segfault on the quic loop thread ~1s after launch (Session iOS 2.15.4, 3 crashes in 20 minutes of Appium runs against devnet).

  • Run the post-refresh callbacks from a moved-out copy of _after_snode_cache_refresh. A deferred get_swarm re-registers itself mid-loop, reallocating the vector being iterated. The old trailing clear() also discarded those re-registrations, so a waiting get_swarm could hang instead.
  • Make _update_cache synchronous via call_get, so its &nodes capture can't dangle. Both callers are already on the loop thread, so no behaviour change.

@mpretty-cyro
mpretty-cyro requested a review from jagerman July 30, 2026 00:54
@mpretty-cyro mpretty-cyro self-assigned this Jul 30, 2026
@mpretty-cyro
mpretty-cyro force-pushed the fix/snode-pool-crash branch from e026b63 to 6f8f38b Compare July 30, 2026 01:36
Fixes a segfault on the quic loop thread ~1s after launch (Session iOS 2.15.4, 3 crashes in 20 minutes of Appium runs against devnet).

- Run the post-refresh callbacks from a moved-out copy of `_after_snode_cache_refresh`. A deferred `get_swarm` re-registers itself mid-loop, reallocating the vector being iterated. The old trailing `clear()` also discarded those re-registrations, so a waiting `get_swarm` could hang instead.
- Make `_update_cache` synchronous via `call_get`, so its `&nodes` capture can't dangle. Both callers are already on the loop thread, so no behaviour change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mpretty-cyro
mpretty-cyro force-pushed the fix/snode-pool-crash branch from 6f8f38b to c7c7a9a Compare July 30, 2026 01:38
@mpretty-cyro
mpretty-cyro force-pushed the fix/snode-pool-crash branch from acdbff1 to b1ba785 Compare July 30, 2026 02:06
@mpretty-cyro
mpretty-cyro enabled auto-merge July 30, 2026 02:23
@mpretty-cyro
mpretty-cyro merged commit 8148d35 into session-foundation:dev Jul 30, 2026
1 check passed
@mpretty-cyro
mpretty-cyro deleted the fix/snode-pool-crash branch July 30, 2026 04:17
jagerman pushed a commit that referenced this pull request Jul 30, 2026
Forward-port of #105 (dev commits c7c7a9a..56afd17) onto pfs.

- Run the post-refresh callbacks from a moved-out copy of
  `_after_snode_cache_refresh`.  A deferred `get_swarm` re-registers itself
  mid-loop, reallocating the vector being iterated.  The old trailing
  `clear()` also discarded those re-registrations, so a waiting `get_swarm`
  could hang instead.
- Make `_update_cache` synchronous via `call_get`, so its `&nodes` capture
  can't dangle.  Both callers are already on the loop thread, so no
  behaviour change.

Line-for-line identical to the dev change apart from adapting the test to
pfs's `_hexbytes` -> `_hex_b` literal rename.
jagerman added a commit that referenced this pull request Jul 30, 2026
dev's only new content since 7e4d764 is #105 (SnodePool post-refresh
callback use-after-free) plus its follow-up comment tweak, forward-ported
to pfs in 984e3a2.  This merge therefore uses `-s ours` to keep pfs's tree
byte-for-byte unchanged; it exists only to record dev as merged so that a
future `git merge pfs` into dev doesn't re-present those hunks as conflicts.
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.

2 participants