De-race types.RoomInfo (#2600)

roomserver/internal/helpers/auth.go

@@ -50,14 +50,14 @@ func CheckForSoftFail(
 		if err != nil {
 			return false, fmt.Errorf("db.RoomNID: %w", err)
 		}
-		if roomInfo == nil || roomInfo.IsStub {
+		if roomInfo == nil || roomInfo.IsStub() {
 			return false, nil
 		}
 
 		// Then get the state entries for the current state snapshot.
 		// We'll use this to check if the event is allowed right now.
 		roomState := state.NewStateResolution(db, roomInfo)
-		authStateEntries, err = roomState.LoadStateAtSnapshot(ctx, roomInfo.StateSnapshotNID)
+		authStateEntries, err = roomState.LoadStateAtSnapshot(ctx, roomInfo.StateSnapshotNID())
 		if err != nil {
 			return true, fmt.Errorf("roomState.LoadStateAtSnapshot: %w", err)
 		}

roomserver/internal/helpers/helpers.go

@@ -409,7 +409,7 @@ func QueryLatestEventsAndState(
 	if err != nil {
 		return err
 	}
-	if roomInfo == nil || roomInfo.IsStub {
+	if roomInfo == nil || roomInfo.IsStub() {
 		response.RoomExists = false
 		return nil
 	}

roomserver/internal/perform/perform_admin.go

@@ -52,7 +52,7 @@ func (r *Admin) PerformAdminEvacuateRoom(
 		}
 		return
 	}
-	if roomInfo == nil || roomInfo.IsStub {
+	if roomInfo == nil || roomInfo.IsStub() {
 		res.Error = &api.PerformError{
 			Code: api.PerformErrorNoRoom,
 			Msg:  fmt.Sprintf("Room %s not found", req.RoomID),

roomserver/internal/perform/perform_backfill.go

@@ -73,7 +73,7 @@ func (r *Backfiller) PerformBackfill(
 	if err != nil {
 		return err
 	}
-	if info == nil || info.IsStub {
+	if info == nil || info.IsStub() {
 		return fmt.Errorf("PerformBackfill: missing room info for room %s", request.RoomID)
 	}
 
@@ -106,7 +106,7 @@ func (r *Backfiller) backfillViaFederation(ctx context.Context, req *api.Perform
 	if err != nil {
 		return err
 	}
-	if info == nil || info.IsStub {
+	if info == nil || info.IsStub() {
 		return fmt.Errorf("backfillViaFederation: missing room info for room %s", req.RoomID)
 	}
 	requester := newBackfillRequester(r.DB, r.FSAPI, r.ServerName, req.BackwardsExtremities, r.PreferServers)
@@ -434,7 +434,7 @@ FindSuccessor:
 		logrus.WithError(err).WithField("room_id", roomID).Error("ServersAtEvent: failed to get RoomInfo for room")
 		return nil
 	}
-	if info == nil || info.IsStub {
+	if info == nil || info.IsStub() {
 		logrus.WithField("room_id", roomID).Error("ServersAtEvent: failed to get RoomInfo for room, room is missing")
 		return nil
 	}

roomserver/internal/perform/perform_inbound_peek.go

@@ -50,7 +50,7 @@ func (r *InboundPeeker) PerformInboundPeek(
 	if err != nil {
 		return err
 	}
-	if info == nil || info.IsStub {
+	if info == nil || info.IsStub() {
 		return nil
 	}
 	response.RoomExists = true

roomserver/internal/perform/perform_invite.go

@@ -125,7 +125,7 @@ func (r *Inviter) PerformInvite(
 		return outputUpdates, nil
 	}
 
-	if (info == nil || info.IsStub) && !isOriginLocal && isTargetLocal {
+	if (info == nil || info.IsStub()) && !isOriginLocal && isTargetLocal {
 		// The invite came in over federation for a room that we don't know about
 		// yet. We need to handle this a bit differently to most invites because
 		// we don't know the room state, therefore the roomserver can't process
@@ -276,7 +276,7 @@ func buildInviteStrippedState(
 	}
 	roomState := state.NewStateResolution(db, info)
 	stateEntries, err := roomState.LoadStateAtSnapshotForStringTuples(
-		ctx, info.StateSnapshotNID, stateWanted,
+		ctx, info.StateSnapshotNID(), stateWanted,
 	)
 	if err != nil {
 		return nil, err

roomserver/internal/query/query.go

@@ -61,7 +61,7 @@ func (r *Queryer) QueryStateAfterEvents(
 	if err != nil {
 		return err
 	}
-	if info == nil || info.IsStub {
+	if info == nil || info.IsStub() {
 		return nil
 	}
 
@@ -302,7 +302,7 @@ func (r *Queryer) QueryServerJoinedToRoom(
 	if err != nil {
 		return fmt.Errorf("r.DB.RoomInfo: %w", err)
 	}
-	if info == nil || info.IsStub {
+	if info == nil || info.IsStub() {
 		return nil
 	}
 	response.RoomExists = true
@@ -390,7 +390,7 @@ func (r *Queryer) QueryMissingEvents(
 	if err != nil {
 		return err
 	}
-	if info == nil || info.IsStub {
+	if info == nil || info.IsStub() {
 		return fmt.Errorf("missing RoomInfo for room %s", events[0].RoomID())
 	}
 
@@ -429,7 +429,7 @@ func (r *Queryer) QueryStateAndAuthChain(
 	if err != nil {
 		return err
 	}
-	if info == nil || info.IsStub {
+	if info == nil || info.IsStub() {
 		return nil
 	}
 	response.RoomExists = true
@@ -774,7 +774,7 @@ func (r *Queryer) QueryRestrictedJoinAllowed(ctx context.Context, req *api.Query
 	if err != nil {
 		return fmt.Errorf("r.DB.RoomInfo: %w", err)
 	}
-	if roomInfo == nil || roomInfo.IsStub {
+	if roomInfo == nil || roomInfo.IsStub() {
 		return nil // fmt.Errorf("room %q doesn't exist or is stub room", req.RoomID)
 	}
 	// If the room version doesn't allow restricted joins then don't
@@ -837,7 +837,7 @@ func (r *Queryer) QueryRestrictedJoinAllowed(ctx context.Context, req *api.Query
 		// See if the room exists. If it doesn't exist or if it's a stub
 		// room entry then we can't check memberships.
 		targetRoomInfo, err := r.DB.RoomInfo(ctx, rule.RoomID)
-		if err != nil || targetRoomInfo == nil || targetRoomInfo.IsStub {
+		if err != nil || targetRoomInfo == nil || targetRoomInfo.IsStub() {
 			res.Resident = false
 			continue
 		}

roomserver/storage/postgres/rooms_table.go

@@ -147,14 +147,16 @@ func (s *roomStatements) InsertRoomNID(
 func (s *roomStatements) SelectRoomInfo(ctx context.Context, txn *sql.Tx, roomID string) (*types.RoomInfo, error) {
 	var info types.RoomInfo
 	var latestNIDs pq.Int64Array
+	var stateSnapshotNID types.StateSnapshotNID
 	stmt := sqlutil.TxStmt(txn, s.selectRoomInfoStmt)
 	err := stmt.QueryRowContext(ctx, roomID).Scan(
-		&info.RoomVersion, &info.RoomNID, &info.StateSnapshotNID, &latestNIDs,
+		&info.RoomVersion, &info.RoomNID, &stateSnapshotNID, &latestNIDs,
 	)
 	if err == sql.ErrNoRows {
 		return nil, nil
 	}
-	info.IsStub = len(latestNIDs) == 0
+	info.SetStateSnapshotNID(stateSnapshotNID)
+	info.SetIsStub(len(latestNIDs) == 0)
 	return &info, err
 }
 

roomserver/storage/shared/room_updater.go

@@ -229,8 +229,8 @@ func (u *RoomUpdater) SetLatestEvents(
 		// Since it's entirely possible that this types.RoomInfo came from the
 		// cache, we should make sure to update that entry so that the next run
 		// works from live data.
-		u.roomInfo.StateSnapshotNID = currentStateSnapshotNID
+		u.roomInfo.SetStateSnapshotNID(currentStateSnapshotNID)
-		u.roomInfo.IsStub = false
+		u.roomInfo.SetIsStub(false)
 		return nil
 	})
 }

roomserver/storage/shared/storage.go

@@ -1032,7 +1032,7 @@ func (d *Database) GetStateEvent(ctx context.Context, roomID, evType, stateKey s
 		return nil, fmt.Errorf("room %s doesn't exist", roomID)
 	}
 	// e.g invited rooms
-	if roomInfo.IsStub {
+	if roomInfo.IsStub() {
 		return nil, nil
 	}
 	eventTypeNID, err := d.EventTypesTable.SelectEventTypeNID(ctx, nil, evType)
@@ -1051,7 +1051,7 @@ func (d *Database) GetStateEvent(ctx context.Context, roomID, evType, stateKey s
 	if err != nil {
 		return nil, err
 	}
-	entries, err := d.loadStateAtSnapshot(ctx, roomInfo.StateSnapshotNID)
+	entries, err := d.loadStateAtSnapshot(ctx, roomInfo.StateSnapshotNID())
 	if err != nil {
 		return nil, err
 	}
@@ -1097,7 +1097,7 @@ func (d *Database) GetStateEventsWithEventType(ctx context.Context, roomID, evTy
 		return nil, fmt.Errorf("room %s doesn't exist", roomID)
 	}
 	// e.g invited rooms
-	if roomInfo.IsStub {
+	if roomInfo.IsStub() {
 		return nil, nil
 	}
 	eventTypeNID, err := d.EventTypesTable.SelectEventTypeNID(ctx, nil, evType)
@@ -1108,7 +1108,7 @@ func (d *Database) GetStateEventsWithEventType(ctx context.Context, roomID, evTy
 	if err != nil {
 		return nil, err
 	}
-	entries, err := d.loadStateAtSnapshot(ctx, roomInfo.StateSnapshotNID)
+	entries, err := d.loadStateAtSnapshot(ctx, roomInfo.StateSnapshotNID())
 	if err != nil {
 		return nil, err
 	}
@@ -1225,10 +1225,10 @@ func (d *Database) GetBulkStateContent(ctx context.Context, roomIDs []string, tu
 			return nil, fmt.Errorf("GetBulkStateContent: failed to load room info for room %s : %w", roomID, err2)
 		}
 		// for unknown rooms or rooms which we don't have the current state, skip them.
-		if roomInfo == nil || roomInfo.IsStub {
+		if roomInfo == nil || roomInfo.IsStub() {
 			continue
 		}
-		entries, err2 := d.loadStateAtSnapshot(ctx, roomInfo.StateSnapshotNID)
+		entries, err2 := d.loadStateAtSnapshot(ctx, roomInfo.StateSnapshotNID())
 		if err2 != nil {
 			return nil, fmt.Errorf("GetBulkStateContent: failed to load state for room %s : %w", roomID, err2)
 		}

roomserver/storage/sqlite3/rooms_table.go

@@ -129,9 +129,10 @@ func (s *roomStatements) SelectRoomIDsWithEvents(ctx context.Context, txn *sql.T
 func (s *roomStatements) SelectRoomInfo(ctx context.Context, txn *sql.Tx, roomID string) (*types.RoomInfo, error) {
 	var info types.RoomInfo
 	var latestNIDsJSON string
+	var stateSnapshotNID types.StateSnapshotNID
 	stmt := sqlutil.TxStmt(txn, s.selectRoomInfoStmt)
 	err := stmt.QueryRowContext(ctx, roomID).Scan(
-		&info.RoomVersion, &info.RoomNID, &info.StateSnapshotNID, &latestNIDsJSON,
+		&info.RoomVersion, &info.RoomNID, &stateSnapshotNID, &latestNIDsJSON,
 	)
 	if err != nil {
 		if err == sql.ErrNoRows {
@@ -143,7 +144,8 @@ func (s *roomStatements) SelectRoomInfo(ctx context.Context, txn *sql.Tx, roomID
 	if err = json.Unmarshal([]byte(latestNIDsJSON), &latestNIDs); err != nil {
 		return nil, err
 	}
-	info.IsStub = len(latestNIDs) == 0
+	info.SetStateSnapshotNID(stateSnapshotNID)
+	info.SetIsStub(len(latestNIDs) == 0)
 	return &info, err
 }
 

roomserver/storage/tables/rooms_table_test.go

@@ -63,12 +63,12 @@ func TestRoomsTable(t *testing.T) {
 
 		roomInfo, err := tab.SelectRoomInfo(ctx, nil, room.ID)
 		assert.NoError(t, err)
-		assert.Equal(t, &types.RoomInfo{
+		expected := &types.RoomInfo{
-			RoomNID:          wantRoomNID,
+			RoomNID:     wantRoomNID,
-			RoomVersion:      room.Version,
+			RoomVersion: room.Version,
-			StateSnapshotNID: 0,
+		}
-			IsStub:           true, // there are no latestEventNIDs
+		expected.SetIsStub(true) // there are no latestEventNIDs
-		}, roomInfo)
+		assert.Equal(t, expected, roomInfo)
 
 		roomInfo, err = tab.SelectRoomInfo(ctx, nil, "!doesnotexist:localhost")
 		assert.NoError(t, err)
@@ -103,12 +103,12 @@ func TestRoomsTable(t *testing.T) {
 
 		roomInfo, err = tab.SelectRoomInfo(ctx, nil, room.ID)
 		assert.NoError(t, err)
-		assert.Equal(t, &types.RoomInfo{
+		expected = &types.RoomInfo{
-			RoomNID:          wantRoomNID,
+			RoomNID:     wantRoomNID,
-			RoomVersion:      room.Version,
+			RoomVersion: room.Version,
-			StateSnapshotNID: 1,
+		}
-			IsStub:           false,
+		expected.SetStateSnapshotNID(1)
-		}, roomInfo)
+		assert.Equal(t, expected, roomInfo)
 
 		eventNIDs, snapshotNID, err := tab.SelectLatestEventNIDs(ctx, nil, wantRoomNID)
 		assert.NoError(t, err)

roomserver/types/types.go

@@ -19,6 +19,7 @@ import (
 	"encoding/json"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
@@ -279,8 +280,33 @@ func (e RejectedError) Error() string { return string(e) }
 
 // RoomInfo contains metadata about a room
 type RoomInfo struct {
+	mu               sync.RWMutex
 	RoomNID          RoomNID
 	RoomVersion      gomatrixserverlib.RoomVersion
-	StateSnapshotNID StateSnapshotNID
+	stateSnapshotNID StateSnapshotNID
-	IsStub           bool
+	isStub           bool
+}
+
+func (r *RoomInfo) StateSnapshotNID() StateSnapshotNID {
+	r.mu.RLock()
+	defer r.mu.RUnlock()
+	return r.stateSnapshotNID
+}
+
+func (r *RoomInfo) IsStub() bool {
+	r.mu.RLock()
+	defer r.mu.RUnlock()
+	return r.isStub
+}
+
+func (r *RoomInfo) SetStateSnapshotNID(nid StateSnapshotNID) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	r.stateSnapshotNID = nid
+}
+
+func (r *RoomInfo) SetIsStub(isStub bool) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	r.isStub = isStub
 }