Use background contexts during federated join for clarity (#2134)

* Use background contexts for clarity

* Don't wait for the context to expire before trying to return

* Actually we don't really need a goroutine here

1 files changed, 29 insertions, 42 deletions

diff --git a/federationapi/internal/perform.go b/federationapi/internal/perform.go
index 4dd53c11..7850f206 100644
--- a/federationapi/internal/perform.go
+++ b/federationapi/internal/perform.go
@@ -196,29 +196,22 @@ func (r *FederationInternalAPI) performJoinUsingServer(
 		return fmt.Errorf("respMakeJoin.JoinEvent.Build: %w", err)
 	}
 
-	// No longer reuse the request context from this point forward.
-	// We don't want the client timing out to interrupt the join.
-	var cancel context.CancelFunc
-	ctx, cancel = context.WithCancel(context.Background())
-
 	// Try to perform a send_join using the newly built event.
 	respSendJoin, err := r.federation.SendJoin(
-		ctx,
+		context.Background(),
 		serverName,
 		event,
 		respMakeJoin.RoomVersion,
 	)
 	if err != nil {
 		r.statistics.ForServer(serverName).Failure()
-		cancel()
 		return fmt.Errorf("r.federation.SendJoin: %w", err)
 	}
 	r.statistics.ForServer(serverName).Success()
 
 	// Sanity-check the join response to ensure that it has a create
 	// event, that the room version is known, etc.
-	if err := sanityCheckAuthChain(respSendJoin.AuthEvents); err != nil {
-		cancel()
+	if err = sanityCheckAuthChain(respSendJoin.AuthEvents); err != nil {
 		return fmt.Errorf("sanityCheckAuthChain: %w", err)
 	}
 
@@ -227,41 +220,35 @@ func (r *FederationInternalAPI) performJoinUsingServer(
 	// to complete, but if the client does give up waiting, we'll
 	// still continue to process the join anyway so that we don't
 	// waste the effort.
-	go func() {
-		defer cancel()
-
-		// TODO: Can we expand Check here to return a list of missing auth
-		// events rather than failing one at a time?
-		respState, err := respSendJoin.Check(ctx, r.keyRing, event, federatedAuthProvider(ctx, r.federation, r.keyRing, serverName))
-		if err != nil {
-			logrus.WithFields(logrus.Fields{
-				"room_id": roomID,
-				"user_id": userID,
-			}).WithError(err).Error("Failed to process room join response")
-			return
-		}
+	// TODO: Can we expand Check here to return a list of missing auth
+	// events rather than failing one at a time?
+	var respState *gomatrixserverlib.RespState
+	respState, err = respSendJoin.Check(
+		context.Background(),
+		r.keyRing,
+		event,
+		federatedAuthProvider(ctx, r.federation, r.keyRing, serverName),
+	)
+	if err != nil {
+		return fmt.Errorf("respSendJoin.Check: %w", err)
+	}
 
-		// If we successfully performed a send_join above then the other
-		// server now thinks we're a part of the room. Send the newly
-		// returned state to the roomserver to update our local view.
-		if err = roomserverAPI.SendEventWithState(
-			ctx, r.rsAPI,
-			roomserverAPI.KindNew,
-			respState,
-			event.Headered(respMakeJoin.RoomVersion),
-			serverName,
-			nil,
-			false,
-		); err != nil {
-			logrus.WithFields(logrus.Fields{
-				"room_id": roomID,
-				"user_id": userID,
-			}).WithError(err).Error("Failed to send room join response to roomserver")
-			return
-		}
-	}()
+	// If we successfully performed a send_join above then the other
+	// server now thinks we're a part of the room. Send the newly
+	// returned state to the roomserver to update our local view.
+	if err = roomserverAPI.SendEventWithState(
+		context.Background(),
+		r.rsAPI,
+		roomserverAPI.KindNew,
+		respState,
+		event.Headered(respMakeJoin.RoomVersion),
+		serverName,
+		nil,
+		false,
+	); err != nil {
+		return fmt.Errorf("roomserverAPI.SendEventWithState: %w", err)
+	}
 
-	<-ctx.Done()
 	return nil
 }