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.
This commit is contained in:
Quentin Gliech 2024-03-06 16:00:20 +01:00 committed by GitHub
parent 2d1bb0b06b
commit 4af33015af
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 21 additions and 16 deletions

1
changelog.d/16973.bugfix Normal file
View file

@ -0,0 +1 @@
Fix joining remote rooms when a module uses the `on_new_event` callback. This callback may now pass partial state events instead of the full state for remote rooms.

View file

@ -142,6 +142,10 @@ Called after sending an event into a room. The module is passed the event, as we
as the state of the room _after_ the event. This means that if the event is a state event, as the state of the room _after_ the event. This means that if the event is a state event,
it will be included in this state. it will be included in this state.
The state map may not be complete if Synapse hasn't yet loaded the full state
of the room. This can happen for events in rooms that were just joined from
a remote server.
Note that this callback is called when the event has already been processed and stored Note that this callback is called when the event has already been processed and stored
into the room, which means this callback cannot be used to deny persisting the event. To into the room, which means this callback cannot be used to deny persisting the event. To
deny an incoming event, see [`check_event_for_spam`](spam_checker_callbacks.md#check_event_for_spam) instead. deny an incoming event, see [`check_event_for_spam`](spam_checker_callbacks.md#check_event_for_spam) instead.

View file

@ -366,7 +366,7 @@ class ThirdPartyEventRulesModuleApiCallbacks:
if len(self._check_threepid_can_be_invited_callbacks) == 0: if len(self._check_threepid_can_be_invited_callbacks) == 0:
return True 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: for callback in self._check_threepid_can_be_invited_callbacks:
try: try:
@ -399,7 +399,7 @@ class ThirdPartyEventRulesModuleApiCallbacks:
if len(self._check_visibility_can_be_modified_callbacks) == 0: if len(self._check_visibility_can_be_modified_callbacks) == 0:
return True 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: for callback in self._check_visibility_can_be_modified_callbacks:
try: try:
@ -427,7 +427,13 @@ class ThirdPartyEventRulesModuleApiCallbacks:
return return
event = await self.store.get_event(event_id) 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: for callback in self._on_new_event_callbacks:
try: try:
@ -490,17 +496,6 @@ class ThirdPartyEventRulesModuleApiCallbacks:
) )
return True 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( async def on_profile_update(
self, user_id: str, new_profile: ProfileInfo, by_admin: bool, deactivation: bool self, user_id: str, new_profile: ProfileInfo, by_admin: bool, deactivation: bool
) -> None: ) -> None:

View file

@ -562,10 +562,15 @@ class StateStorageController:
@trace @trace
@tag_args @tag_args
async def get_current_state( 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]: ) -> StateMap[EventBase]:
"""Same as `get_current_state_ids` but also fetches the events""" """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())) event_map = await self.stores.main.get_events(list(state_map_ids.values()))