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

user_left_room distributor signal is unreliable #13673

Open
squahtx opened this issue Aug 30, 2022 · 2 comments
Open

user_left_room distributor signal is unreliable #13673

squahtx opened this issue Aug 30, 2022 · 2 comments
Labels
A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. A-Membership O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@squahtx
Copy link
Contributor

squahtx commented Aug 30, 2022

In Synapse, there is a class called Distributor, which maintains a list of callbacks for a named signal and calls them all when the signal is fired: https://github.com/matrix-org/synapse/blob/develop/synapse/util/distributor.py#L43

There's only one signal defined currently and it's user_left_room, which is used in two places:

  1. DeviceHandler, to mark a remote user's device list as unsubscribed
    async def user_left_room(self, user: UserID, room_id: str) -> None:
    user_id = user.to_string()
    room_ids = await self.store.get_rooms_for_user(user_id)
    if not room_ids:
    # We no longer share rooms with this user, so we'll no longer
    # receive device updates. Mark this in DB.
    await self.store.mark_remote_user_device_list_as_unsubscribed(user_id)
  2. TypingWriterHandler, to stop a user's typing indicator when they leave a room.
    def user_left_room(self, user: UserID, room_id: str) -> None:
    user_id = user.to_string()
    if self.is_mine_id(user_id):
    member = RoomMember(room_id=room_id, user_id=user_id)
    self._stopped_typing(member)

Unfortunately, the user_left_room signal is currently only reliable when local users leave a room normally (and not via a ban!).
This manifested as #13651 for device lists. Once #13651 is fixed, device list tracking will no longer use the signal.

We should look into whether the typing listener is necessary and if it can be moved elsewhere, so that we can eliminate this bit of architecture that's only used by one tiny part of synapse.

@squahtx squahtx added A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. A-Membership O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Aug 30, 2022
@DMRobertson
Copy link
Contributor

The Distributor sounds a lot like the Notifier:

class Notifier:

@squahtx
Copy link
Contributor Author

squahtx commented Sep 22, 2022

There's only one signal defined currently and it's user_left_room, which is used in two places:
2. TypingWriterHandler, to stop a user's typing indicator when they leave a room.

related:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. A-Membership O-Uncommon Most users are unlikely to come across this or unexpected workflow 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

No branches or pull requests

2 participants