Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

_should_perform_remote_join inspects the current state of the room, not the state of the would-be join #13269

Closed
DMRobertson opened this issue Jul 13, 2022 · 0 comments · Fixed by #13270
Assignees
Labels
S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@DMRobertson
Copy link
Contributor

DMRobertson commented Jul 13, 2022

In normal operation (<= 10 fwd extremities, no races over the relevant state) the two state maps would be one and the same. But they need not be. For consistency's sake we should run any state-dependent checks against the state prior to the would-be join. That's what other HSes will do!

Additionally, the current state of the room is cached in-application and therefore subject to cache invalidation races. By using the state at the would-be join we avoid this race. This explains a particularly gnarly complement flake (xref #13161):

  1. Alice invites Bob to a restricted room that Bob would otherwise be unable to join.
  2. Bob tries to join the room. We select prev_events for Bob's join; resolve across them to produce the state before this join; and determine that his membership is invite.
  3. Bob's HS calls _should_perform_remote_join to work out if it can needs to use a remote make_join/send_join handshake. To do so, it requests the current state of the room. This is cached and subject to a cache invalidation race; the cached current state may not have been updated to include the fact that Bob is invited.
@DMRobertson DMRobertson added S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Jul 13, 2022
@DMRobertson DMRobertson self-assigned this Jul 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant