Outbound group sessions are invalidated upon reception of a duplicate state event
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 35/100
Research direction
Start with update_state() in mautrix/client/state_store/abstract.py and its registration in mautrix/client/client.py, then trace handle_member_event() in mautrix/crypto/machine.py and its registration there. Compare the concurrent loading and comparison paths with the referenced commits. Done means duplicate membership events no longer invalidate outbound group sessions while genuine state changes still do.
Written by the indexing model from the issue text.
Description
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 72fb0f61f2a8cf8f062ddb8bacaa98ef4cbd0aa5. 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:
https://github.com/mautrix/python/blob/3e22e6192e2a9d208b9e7ea159b2169daa60a1ce/mautrix/client/state_store/abstract.py#L146-L148
https://github.com/mautrix/python/blob/3e22e6192e2a9d208b9e7ea159b2169daa60a1ce/mautrix/crypto/machine.py#L193-L200
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:
https://github.com/mautrix/python/blob/3e22e6192e2a9d208b9e7ea159b2169daa60a1ce/mautrix/client/client.py#L31
Comparing the received membership event against the most recent one happens in handle_member_event(), which is registered here:
https://github.com/mautrix/python/blob/3e22e6192e2a9d208b9e7ea159b2169daa60a1ce/mautrix/crypto/machine.py#L109
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 https://github.com/jkhsjdhjs/mautrix-python/commit/e7c1921fb8f52549baf09ed2784fe46573964911, as I don't know a good way to fix this properly.
- Dominant language
- Python
- Stars
- 249
- Forks
- 84
- PR merge metrics
- No merged PRs in 30d
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from mautrix/python
-
Difficulty 5/5 Over a week Newbie friendliness 30/100
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 45/100
-
Difficulty 5/5 Over a week Newbie friendliness 25/100
-
Difficulty 4/5 3-5 days Newbie friendliness 30/100
-
Difficulty 4/5 3-5 days Newbie friendliness 38/100
Similar issues
-
documentation help wanted
Difficulty 2/5 1-3 hours Newbie friendliness 90/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 90/100
simonw/sqlite-utils#872 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100