Fix cross-test pollution from engineered homeservers re-using the same server names (fix TestPartialStateJoin flakes) (#880)

### What problem are we trying to solve?

This PR is tackling this problem:

> Many of the faster joins test flakes are due to the homeserver under test failing to contact Complement homeservers after they have been torn down. When this happens, subsequent tests can fail if they use a Complement homeserver that happens to have the same `hostname:port` as one which the homeserver under test has previously marked as offline.
>
> *-- #626

This problem occurs because as an [optimization](#471) we try to share the `deployment` across many tests. To be clear, as a dumb-simple solution, if we created a new `deployment` for each test, we wouldn't run into this issue (but then the tests would be "slow" again).


### What does this PR do?

Spawning from #878 (comment) where I originally thought we could prevent stray requests to a server just from the federation request [authentication](https://spec.matrix.org/v1.18/server-server-api/#authentication) but turns out, it's already doing the right thing. The only thing we can do is compare the `destination` to our server name which [`VerifyHTTPRequest`](https://github.com/matrix-org/gomatrixserverlib/blob/c9c468727353c4835523349dc7c202e5de9a2e37/fclient/request.go#L216-L227) is already doing. And since the crux of this whole thing is that two Complement engineered servers can use the same recycled port and have identical server names (`host.docker.internal:port`), it doesn't help us here. The rest of the public key stuff that is part of federation requests is just for the origin server to sign itself with, for authenticity.

> `destination`: [Added in v1.3] the server name of the receiving server. This is the same as the `destination` field from the JSON described in step 1. For compatibility with older servers, recipients should accept requests without this parameter, but MUST always send it. If this property is included, but the value does not match the receiving server’s name, the receiving server must deny the request with an HTTP status code 401 Unauthorized.
>
> *-- https://spec.matrix.org/v1.18/server-server-api/#authentication*

Instead, this PR takes an alternative route where we instead never re-use the same `server_name` for a Complement engineered homeserver. This means that even if the real homeserver reaches out to the torn-down engineered homeservers, it just hits 'connection refused' instead of a live, unrelated server. No cross-test pollution.

In the context of `tests/msc3902/federation_room_join_partial_state_test.go`, it means the engineered homeserver will never see a [`t.Errorf("Received unexpected PDU", ...)`](https://github.com/matrix-org/complement/blob/bc2b638b11f5b0a068e39ae51f7ac2782070d14f/tests/msc3902/federation_room_join_partial_state_test.go#L77C7-L77C40) problem.


### How do we currently deal with this?

Currently, we try to workaround the stray request problem by making sure the engineered homeserver leaves any rooms it shares with the real homeserver. This means the real homeserver has no logical reason to contact the other server but it's still technically possible that it might reach out because of some queued federation traffic, etc. I find this approach/pattern pretty bad (especially in the way it's currently implemented). See the "Fix" explanation on #878 for more info.

See all of the logic around [`partialStateJoinResult.Destroy`](https://github.com/matrix-org/complement/blob/bc2b638b11f5b0a068e39ae51f7ac2782070d14f/tests/msc3902/federation_room_join_partial_state_test.go#L4399-L4428) and [`WithWaitForLeave`](https://github.com/matrix-org/complement/blob/bc2b638b11f5b0a068e39ae51f7ac2782070d14f/tests/msc3902/federation_room_join_partial_state_test.go#L126-L162) to see how messy this gets and all of the logic that we get to remove because of this new approach.

Author: MadLittleMods

federation/server.go

@@ -528,18 +528,96 @@ func (s *Server) Mux() *mux.Router {
 	return s.mux
 }
 
-// Listen for federation server requests - call the returned function to gracefully close the server.
+// Keep track of the ports that we've previously used so that we never use the same port
+// (and therefore the same `server_name`) to two different servers.
+//
+// A `Server` is identified over federation solely by its `server_name` (which looks
+// like `hostname:port` for these Complement engineered homeservers). When the OS recycles a
+// freed port, a new Server could otherwise get a `server_name` that is identical to a
+// previously torn-down one.
+//
+// To explain an actual situation where this becomes a problem: A real homeserver under
+// test (that is participating in a room with the now-dead engineered homeserver) might
+// still try to reach the dead server, but since the `server_name` is the same, it's now
+// hitting the new server unexpectedly (cross-test pollution).
+//
+// This particularly happens when you try to share a `deployment` across many tests and
+// then each test creates a engineered homeservers to interact against.
+//
+// Retiring each port for the lifetime of the process keeps stray requests pointed at a
+// dead port (connection refused) instead of a live, unrelated server.
+var (
+	// Use a mutex so only one thread can advance `lastUsedPort` at a time. We don't want
+	// multiple threads clobbering `lastUsedPort`.
+	lastUsedPortMu sync.Mutex
+	// Start at 1024 (1023 + 1) to avoid the priviged ports used by the system
+	//
+	// Since we sequentially try each port, we just need to keep track of the last one we tried
+	lastUsedPort = 1023
+)
+
+// listenOnUnusedPort listens on an unused port that no other federation `Server` has
+// used before in this process.
+func listenOnUnusedPort(t ct.TestLike) net.Listener {
+	lastUsedPortMu.Lock()
+	defer lastUsedPortMu.Unlock()
+
+	// We use this sequential port scan strategy over guess and check with an OS-assigned
+	// port (by using `:0`) as it's more efficient. The OS may recycle and re-use freed
+	// ports meaning we could regress to O(n^2) behavior trying to search for each new
+	// port we want to find.
+	//
+	// Using `:0` means an unused port is automatically picked for us (could be random,
+	// could be the next sequential unused port, we don't know). Ideally, we could ask for
+	// the next unused port after X to avoid a bunch of work. When using `:0`, the
+	// pathological case that is O(n^2) is if OS hands back next lowest unused port
+	// sequentially which would mean we would have to probe and hold each listener until
+	// we finally got something new.
+
+	// Try the whole port range (untested but it's probably fast to do so)
+	max_attempts := 65535
+	var lastErr error
+	for i := 0; i < max_attempts; i++ {
+		port := lastUsedPort + 1
+		if port > 65535 {
+			// If this ever becomes a problem, we can namespace used ports by `deployment` since
+			// that has to be passed into `NewServer(...)` anyway and the whole point of this is
+			// that a homeserver from the `deployment` doesn't try to reach out to a previous
+			// engineered homeserver it knows about.
+			//
+			// As another alternative, we could also wrap-around to the beginning of the port
+			// range again although that is slightly unsound.
+			ct.Fatalf(
+				t, "listenOnUnusedPort: could not find an unused port in the entire port range (0 - 65535). "+
+					"(see comment here if you run into this). Last error: %s", lastErr,
+			)
+		}
+
+		// Check port availability
+		ln, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
+		lastUsedPort = port
+		if err != nil {
+			lastErr = err
+			// Port unavailable, skip
+			continue
+		}
+
+		return ln
+	}
+	// Since we try the entire port range, we don't really expect to get here but we have
+	// it in case there is a programming error above
+	ct.Fatalf(t, "listenOnUnusedPort: Programming error")
+	return nil
+}
+
 func (s *Server) Listen() (cancel func()) {
 	if s.listening {
 		return
 	}
 	var wg sync.WaitGroup
 	wg.Add(1)
 
-	ln, err := net.Listen("tcp", ":0") //nolint
-	if err != nil {
-		ct.Fatalf(s.t, "ListenFederationServer: net.Listen failed: %s", err)
-	}
+	ln := listenOnUnusedPort(s.t)
 	port := ln.Addr().(*net.TCPAddr).Port
 	s.serverName = spec.ServerName(fmt.Sprintf("%s:%d", s.serverName, port))
 	s.listening = true