summary refs log tree commit diff
path: root/synapse
diff options
context:
space:
mode:
authorQuentin Gliech <quenting@element.io>2024-03-06 16:00:20 +0100
committerGitHub <noreply@github.com>2024-03-06 16:00:20 +0100
commit4af33015af280b4716e812e47d5631fcac088128 (patch)
tree45e0633969e26f4137a539134bc63ab59925ffdb /synapse
parentMerge remote-tracking branch 'origin/release-v1.102' into develop (diff)
downloadsynapse-4af33015af280b4716e812e47d5631fcac088128.tar.xz
Fix joining remote rooms when a `on_new_event` callback is registered (#16973)
Since Synapse 1.76.0, any module which registers a `on_new_event`
callback would brick the ability to join remote rooms.
This is because this callback tried to get the full state of the room,
which would end up in a deadlock.

Related:
https://github.com/matrix-org/synapse-auto-accept-invite/issues/18

The following module would brick the ability to join remote rooms:

```python
from typing import Any, Dict, Literal, Union
import logging

from synapse.module_api import ModuleApi, EventBase

logger = logging.getLogger(__name__)

class MyModule:
    def __init__(self, config: None, api: ModuleApi):
        self._api = api
        self._config = config

        self._api.register_third_party_rules_callbacks(
            on_new_event=self.on_new_event,
        )

    async def on_new_event(self, event: EventBase, _state_map: Any) -> None:
        logger.info(f"Received new event: {event}")

    @staticmethod
    def parse_config(_config: Dict[str, Any]) -> None:
        return None
```

This is technically a breaking change, as we are now passing partial
state on the `on_new_event` callback.
However, this callback was broken for federated rooms since 1.76.0, and
local rooms have full state anyway, so it's unlikely that it would
change anything.
Diffstat (limited to 'synapse')
-rw-r--r--synapse/module_api/callbacks/third_party_event_rules_callbacks.py23
-rw-r--r--synapse/storage/controllers/state.py9
2 files changed, 16 insertions, 16 deletions
diff --git a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py

index 7a3255aeef..9f7a04372d 100644 --- a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py +++ b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py
@@ -366,7 +366,7 @@ class ThirdPartyEventRulesModuleApiCallbacks: if len(self._check_threepid_can_be_invited_callbacks) == 0: return True - state_events = await self._get_state_map_for_room(room_id) + state_events = await self._storage_controllers.state.get_current_state(room_id) for callback in self._check_threepid_can_be_invited_callbacks: try: @@ -399,7 +399,7 @@ class ThirdPartyEventRulesModuleApiCallbacks: if len(self._check_visibility_can_be_modified_callbacks) == 0: return True - state_events = await self._get_state_map_for_room(room_id) + state_events = await self._storage_controllers.state.get_current_state(room_id) for callback in self._check_visibility_can_be_modified_callbacks: try: @@ -427,7 +427,13 @@ class ThirdPartyEventRulesModuleApiCallbacks: return event = await self.store.get_event(event_id) - state_events = await self._get_state_map_for_room(event.room_id) + + # We *don't* want to wait for the full state here, because waiting for full + # state will persist event, which in turn will call this method. + # This would end up in a deadlock. + state_events = await self._storage_controllers.state.get_current_state( + event.room_id, await_full_state=False + ) for callback in self._on_new_event_callbacks: try: @@ -490,17 +496,6 @@ class ThirdPartyEventRulesModuleApiCallbacks: ) return True - async def _get_state_map_for_room(self, room_id: str) -> StateMap[EventBase]: - """Given a room ID, return the state events of that room. - - Args: - room_id: The ID of the room. - - Returns: - A dict mapping (event type, state key) to state event. - """ - return await self._storage_controllers.state.get_current_state(room_id) - async def on_profile_update( self, user_id: str, new_profile: ProfileInfo, by_admin: bool, deactivation: bool ) -> None: diff --git a/synapse/storage/controllers/state.py b/synapse/storage/controllers/state.py
index 99666d79a9..22d93a561c 100644 --- a/synapse/storage/controllers/state.py +++ b/synapse/storage/controllers/state.py
@@ -562,10 +562,15 @@ class StateStorageController: @trace @tag_args async def get_current_state( - self, room_id: str, state_filter: Optional[StateFilter] = None + self, + room_id: str, + state_filter: Optional[StateFilter] = None, + await_full_state: bool = True, ) -> StateMap[EventBase]: """Same as `get_current_state_ids` but also fetches the events""" - state_map_ids = await self.get_current_state_ids(room_id, state_filter) + state_map_ids = await self.get_current_state_ids( + room_id, state_filter, await_full_state + ) event_map = await self.stores.main.get_events(list(state_map_ids.values()))