diff --git a/src/github.com/matrix-org/dendrite/roomserver/input/latest_events.go b/src/github.com/matrix-org/dendrite/roomserver/input/latest_events.go index c3a8c624e..ee61cf5aa 100644 --- a/src/github.com/matrix-org/dendrite/roomserver/input/latest_events.go +++ b/src/github.com/matrix-org/dendrite/roomserver/input/latest_events.go @@ -6,6 +6,7 @@ import ( "github.com/matrix-org/gomatrixserverlib" ) +// updateLatestEvents updates the list of latest events for this room. func updateLatestEvents( db RoomEventDatabase, roomNID types.RoomNID, stateAtEvent types.StateAtEvent, event gomatrixserverlib.Event, ) error { @@ -14,8 +15,15 @@ func updateLatestEvents( return err } defer func() { - // Commit if there wasn't an error. - updater.Close(err == nil) + if err == nil { + // Commit if there wasn't an error. + // Set the returned err value if we encounter an error committing. + err = updater.Close(true) + } else { + // Ignore any error we get rolling back since we don't want to + // clobber the current error + updater.Close(false) + } }() var prevEvents []gomatrixserverlib.EventReference @@ -54,11 +62,11 @@ func updateLatestEvents( }) } - err = updater.SetLatestEvents(roomNID, newLatest) - if err != nil { + if err = updater.SetLatestEvents(roomNID, newLatest); err != nil { return err } - // The deferred fires and the transaction closes. - return nil + // The err should be nil at this point. + // But when we call Close in the defer above it might set an error here. + return err } diff --git a/src/github.com/matrix-org/dendrite/roomserver/storage/previous_events_table.go b/src/github.com/matrix-org/dendrite/roomserver/storage/previous_events_table.go index e7d2d039e..2d081f936 100644 --- a/src/github.com/matrix-org/dendrite/roomserver/storage/previous_events_table.go +++ b/src/github.com/matrix-org/dendrite/roomserver/storage/previous_events_table.go @@ -21,7 +21,11 @@ CREATE TABLE IF NOT EXISTS previous_events ( ); ` -// Insert an entry of the prev event or if the prev_event doesn't exist. +// Insert an entry into the previous_events table. +// The there is already an entry indicating that an event references that previous event then +// add the event NID to the list to indicate that this event references that previous event as well. +// This should only be modified while holding a "FOR UPDATE" lock on the row in the rooms table for this room. +// The lock is necessary to avoid data races when checking whether an event is already referenced by another event. const insertPreviousEventSQL = "" + "INSERT INTO previous_events VALUES" + " (previous_event_id, previous_reference_sha256, event_nids)" + @@ -30,6 +34,8 @@ const insertPreviousEventSQL = "" + " DO UPDATE SET event_nids = array_append(previous_events.event_nids, $3)" + " WHERE $3 != ANY(previous_events.event_nids)" +// Check if the event is referenced by another event in the table. +// This should only be done while holding a "FOR UPDATE" lock on the row in the rooms table for this room. const selectPreviousEventExistsSQL = "" + "SELECT 1 FROM previous_events" + " WHERE previous_event_id = $1 AND previous_reference_sha256 = $2"