Fix a rare bug where initial /syncs would fail

[squahtx]

This change fixes a rare bug where initial /syncs would fail with a
KeyError under the following circumstances:

1. A user fast joins a remote room.
2. The user is kicked from the room before the room's full state has
   been synced.
3. A second local user fast joins the room.
4. Events are backfilled into the room with a higher topological
   ordering than the original user's leave. They are assigned a
   negative stream ordering. It's not clear how backfill happened here,
   since it is expected to be equivalent to syncing the full state.
5. The second local user leaves the room before the room's full state
   has been synced. The homeserver does not complete the sync.
6. The original user performs an initial /sync with lazy_load_members
   enabled.
   • Because they were kicked from the room, the room is included in
     the /sync response even though the include_leave option is not
     specified.
   • To populate the room's timeline, _load_filtered_recents /
     get_recent_events_for_room fetches events with a lower stream
     ordering than the leave event and picks the ones with the highest
     topological orderings (which are most recent). This captures the
     backfilled events after the leave, since they have a negative
     stream ordering. These events are filtered out of the timeline,
     since the user was not in the room at the time and cannot view
     them. The sync code ends up with an empty timeline for the room
     that notably does not include the user's leave event.
     This seems buggy, but at least we don't disclose events the user
     isn't allowed to see.
   • Normally, compute_state_delta would fetch the state at the
     start and end of the room's timeline to generate the sync
     response. Since the timeline is empty, it fetches the state at
     min(now, last event in the room), which corresponds with the
     second user's leave. The state during the entirety of the second
     user's membership does not include the membership for the first
     user because of partial state.
     This part is also questionable, since we are fetching state from
     outside the bounds of the user's membership.
   • compute_state_delta then tries and fails to find the user's
     membership in the auth events of timeline events. Because there
     is no timeline event whose auth events are expected to contain
     the user's membership, a KeyError is raised.

Also contains a drive-by fix for a separate unlikely race condition.

Signed-off-by: Sean Quah [email protected]

[DMRobertson]

This looks like it was very painful to track down.

[DMRobertson]

I guess to make this 100% robust we'd need to atomically fetch is_partial_state_room while we fetch the state ids?

(To be explicit, I don't think we should do this.)

[squahtx]

That would work, yes. I wonder if partial state-dness should really be a property of state rather than events or rooms.