Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Outbound group sessions are invalidated upon reception of a duplicate state event #169

Open
jkhsjdhjs opened this issue Feb 25, 2024 · 0 comments

Comments

@jkhsjdhjs
Copy link

Mautrix uses lazy membership loading, which causes synapse to oftentimes send state events to the client, that the client has already received. Upon reception of a state event, mautrix invalidates all outbound group sessions. This is the correct thing to do, but because synapse frequently sends duplicate state events, mautrix invalidates outbound group sessions all the time.

A fix for this has first been implemented in 72fb0f6. This fix relies on storing the most recent membership event as an attribute of the event and then later comparing this stored event to the received event and ignoring the received event if they are equal:

evt.unsigned["mautrix_prev_membership"] = await self.get_member(
evt.room_id, UserID(evt.state_key)
)

prev_cache = evt.unsigned.get("mautrix_prev_membership")
if isinstance(prev_cache, Member) and prev_cache.membership == cur:
self.log.debug(
f"Got duplicate membership state event in {evt.room_id} changing {evt.state_key} "
f"from {prev} to {cur}, cached state was {prev_cache} (event ID: {evt.event_id}, "
f"sync source: {src})"
)
return

The problem is, that loading the most recent membership event and comparing the stored membership event happens in different event handlers, which are executed concurrently.
The most recent membership event is loaded in the update_state() event handler, which is registered here:

self.add_event_handler(EventType.ALL, self._update_state)

Comparing the received membership event against the most recent one happens in handle_member_event(), which is registered here:

self.client.add_event_handler(EventType.ROOM_MEMBER, self.handle_member_event)

While the update_state() handler is executed first, execution is interrupted once the most recent membership event is loaded via await self.get_member(), as this loads the event from the database, which takes some time. In the meantime, handle_member_event() becomes active and attempts to compare the received membership event against the most recent one, which hasn't yet been loaded from the database. As a result, the comparison fails and the outbound session is invalidated.

I implemented a hacky fix for this in jkhsjdhjs@e7c1921, as I don't know a good way to fix this properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant