Skip to content

Stop an in-flight poll undoing a cursor reset - #2016

Merged
mpretty-cyro merged 1 commit into
session-foundation:devfrom
mpretty-cyro:fix/cursor-reset-race
Sep 28, 2026
Merged

mpretty-cyro merged 1 commit into
session-foundation:devfrom
mpretty-cyro:fix/cursor-reset-race

Conversation

@mpretty-cyro

Copy link
Copy Markdown
Collaborator

The bug

A group's poll cursor (its per-snode last hashes) is reset so this device re-fetches the group's
history. If a poll of that swarm is in flight when the reset happens, it finishes afterwards and
writes its newest hash back as the cursor. That undoes the reset, and the history is never fetched.
The window is one poll round trip. It is real because promotion arrives in our own swarm while the
group is polled on its own schedule.

pollNodeForKey already had a check for this, but it could never fire. It tested the
{ namespace, lastHash } objects, which are always truthy, instead of their lastHash:

namespacesAndLastHashes.some(m => m) && namespacesAndLastHashesAfterFetch.every(m => !m)

Fixing it to read lastHash would still miss two cases:

  • a reset of a cursor that was already empty, where both reads are empty;
  • a reset that lands while the cursor write is itself in progress.

Reset sites

All three go through SwarmPolling.resetLastHashesForConversation, which is where the guard sits,
so a future caller is covered too:

  • Promotion to admin: handleGroupUpdatePromoteMessage, handleGroupV2Message.ts:621.
  • Recreating a missing group dump: createInitialDumpsMissingForGroups,
    libsession_utils.ts:735.
  • Deleting a group with clearFetchedHashes: ConvoHub.deleteGroup,
    ConversationController.ts:442. It is reached from:
    • leaving a group (useShowLeaveGroup.ts);
    • the group being removed from user groups (configMessage.ts:706);
    • being kicked (handleLibSessionMessage.ts:60);
    • a group missing from the wrapper after a poll (swarmPolling.ts:825);
    • a group destroyed after a merge (SwarmPollingGroupConfig.ts:58);
    • metaGroups.ts:283.

The guard

  • Each reset bumps a per-conversation count, synchronously, before its first await.
  • A poll takes the count when it starts. If the count has changed by the time its fetch returns,
    it writes no cursor and drops its results.
  • The count is checked again before each part of the cursor write (database and in-memory cache).
    A reset landing during those awaits is therefore not undone either.

Dropping the in-flight poll's messages keeps the discard the old check intended. The next poll
fetches them again from the start, and seen-message dedupe absorbs the overlap.

A reset that happens after the poll's cursor write has landed is outside this fix.

Testing

SwarmPolling_cursorReset_test.ts drives pollOnceForKey with both polled snodes' retrieves held
open, resets the cursor, then lets them complete. The cases:

  • the cursor was already empty before the fetch;
  • the cursor was set before the fetch;
  • the reset lands during the cursor write;
  • a control with no reset, where the poll writes its cursor as before.

Each reset case asserts that no cursor was written and that the next poll asks from the beginning.

957 passing, 0 failing, tsc 0 errors, eslint clean at 7e87828d4.

Mutation results:

guard removed result
all of it 3 failing
the upstream value comparison, with lastHash fixed, in place of the count 2 failing (the empty-cursor and mid-write cases)
only the checks inside the cursor write 1 failing

A group's poll cursor is reset so the device re-fetches its history: on
promotion to admin, on removal or deletion, and when a missing group dump is
recreated. A poll of that swarm still in flight wrote its newest hash back
afterwards and undid the reset, so the history was never fetched.

The check meant to catch this could never fire: it tested the
{namespace, lastHash} objects, which are always truthy, rather than their
lastHash. Comparing values would still miss a reset of an already empty
cursor, so each reset now bumps a per-conversation count. A poll that sees the
count change since it started drops its results and writes no cursor, and the
count is checked again before each part of the cursor write.
@mpretty-cyro
mpretty-cyro marked this pull request as ready for review September 28, 2026 06:00

@Bilb Bilb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@mpretty-cyro
mpretty-cyro merged commit d1fd7e4 into session-foundation:dev Sep 28, 2026
11 checks passed
@mpretty-cyro
mpretty-cyro deleted the fix/cursor-reset-race branch September 28, 2026 06:44
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