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

Commit ae7858f

Browse files
authored
Fix race when persisting an event and deleting a room (#12594)
This works by taking a row level lock on the `rooms` table at the start of both transactions, ensuring that they don't run at the same time. In the event persistence transaction we also check that there is an entry still in the `rooms` table. I can't figure out how to do this in SQLite. I was just going to lock the table, but it seems that we don't support that in SQLite either, so I'm *really* confused as to how we maintain integrity in SQLite when using `lock_table`....
1 parent 01dcf75 commit ae7858f

File tree

3 files changed

+22
-2
lines changed

3 files changed

+22
-2
lines changed

changelog.d/12594.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix race when persisting an event and deleting a room that could lead to outbound federation breaking.

synapse/storage/databases/main/events.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
)
4848
from synapse.storage.databases.main.events_worker import EventCacheEntry
4949
from synapse.storage.databases.main.search import SearchEntry
50+
from synapse.storage.engines.postgres import PostgresEngine
5051
from synapse.storage.util.id_generators import AbstractStreamIdGenerator
5152
from synapse.storage.util.sequence import SequenceGenerator
5253
from synapse.types import StateMap, get_domain_from_id
@@ -364,6 +365,20 @@ def _persist_events_txn(
364365
min_stream_order = events_and_contexts[0][0].internal_metadata.stream_ordering
365366
max_stream_order = events_and_contexts[-1][0].internal_metadata.stream_ordering
366367

368+
# We check that the room still exists for events we're trying to
369+
# persist. This is to protect against races with deleting a room.
370+
#
371+
# Annoyingly SQLite doesn't support row level locking.
372+
if isinstance(self.database_engine, PostgresEngine):
373+
for room_id in {e.room_id for e, _ in events_and_contexts}:
374+
txn.execute(
375+
"SELECT room_version FROM rooms WHERE room_id = ? FOR SHARE",
376+
(room_id,),
377+
)
378+
row = txn.fetchone()
379+
if row is None:
380+
raise Exception(f"Room does not exist {room_id}")
381+
367382
# stream orderings should have been assigned by now
368383
assert min_stream_order
369384
assert max_stream_order

synapse/storage/databases/main/purge_events.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,12 @@ async def purge_room(self, room_id: str) -> List[int]:
324324
)
325325

326326
def _purge_room_txn(self, txn: LoggingTransaction, room_id: str) -> List[int]:
327-
# First we fetch all the state groups that should be deleted, before
327+
# We *immediately* delete the room from the rooms table. This ensures
328+
# that we don't race when persisting events (as that transaction checks
329+
# that the room exists).
330+
txn.execute("DELETE FROM rooms WHERE room_id = ?", (room_id,))
331+
332+
# Next, we fetch all the state groups that should be deleted, before
328333
# we delete that information.
329334
txn.execute(
330335
"""
@@ -403,7 +408,6 @@ def _purge_room_txn(self, txn: LoggingTransaction, room_id: str) -> List[int]:
403408
"room_stats_state",
404409
"room_stats_current",
405410
"room_stats_earliest_token",
406-
"rooms",
407411
"stream_ordering_to_exterm",
408412
"users_in_public_rooms",
409413
"users_who_share_private_rooms",

0 commit comments

Comments
 (0)