From: Bryan C. Mills <bcmills@google.com>
Date: Wed, 22 Mar 2023 16:19:23 +0000 (-0400)
Subject: net/http: improve logging in TestServerSetKeepAlivesEnabledClosesConns
X-Git-Tag: go1.21rc1~1179
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d35dd190ff30fcbab6909ac68ae5114b157b9009;p=gostls13.git

net/http: improve logging in TestServerSetKeepAlivesEnabledClosesConns

- Log the actual addresses reported, in case that information is relevant.

- Keep going after the first error, so that we report more information
  about the idle connections after they have been used. (Was the first
  connection dropped completely, or did it later show up as idle?)

- Remove the third request at the end of the test. It had been
  assuming that the address for a new connection would always be
  different from the address for the just-closed connection; however,
  that assumption does not hold in general.

Removing the third request addresses one of the two failure modes seen
in #55195. It may help in investigating the other failure mode, but I
do not expect it to fix the failures entirely. (I suspect that the
other failure mode is a synchronization bug in returning the idle
connection from the first request.)

For #55195.

Change-Id: If9604ea68db0697268288ce9812dd57633e83fbd
Reviewed-on: https://go-review.googlesource.com/c/go/+/478515
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
---

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index eb4660f8d6..88184bcf35 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -5421,39 +5421,36 @@ func testServerSetKeepAlivesEnabledClosesConns(t *testing.T, mode testMode) {
 	get := func() string { return get(t, c, ts.URL) }
 
 	a1, a2 := get(), get()
-	if a1 != a2 {
-		t.Fatal("expected first two requests on same connection")
+	if a1 == a2 {
+		t.Logf("made two requests from a single conn %q (as expected)", a1)
+	} else {
+		t.Errorf("server reported requests from %q and %q; expected same connection", a1, a2)
 	}
-	addr := strings.TrimPrefix(ts.URL, "http://")
 
 	// The two requests should have used the same connection,
 	// and there should not have been a second connection that
 	// was created by racing dial against reuse.
 	// (The first get was completed when the second get started.)
-	n := tr.IdleConnCountForTesting("http", addr)
-	if n != 1 {
-		t.Fatalf("idle count for %q after 2 gets = %d, want 1", addr, n)
+	if conns := tr.IdleConnStrsForTesting(); len(conns) != 1 {
+		t.Errorf("found %d idle conns (%q); want 1", len(conns), conns)
 	}
 
 	// SetKeepAlivesEnabled should discard idle conns.
 	ts.Config.SetKeepAlivesEnabled(false)
 
-	var idle1 int
 	waitCondition(t, 10*time.Millisecond, func(d time.Duration) bool {
-		idle1 = tr.IdleConnCountForTesting("http", addr)
-		if idle1 != 0 {
+		if conns := tr.IdleConnStrsForTesting(); len(conns) > 0 {
 			if d > 0 {
-				t.Logf("idle count %v after SetKeepAlivesEnabled called = %v; waiting for 0", d, idle1)
+				t.Logf("idle conns %v after SetKeepAlivesEnabled called = %q; waiting for empty", d, conns)
 			}
 			return false
 		}
 		return true
 	})
 
-	a3 := get()
-	if a3 == a2 {
-		t.Fatal("expected third request on new connection")
-	}
+	// If we make a third request it should use a new connection, but in general
+	// we have no way to verify that: the new connection could happen to reuse the
+	// exact same ports from the previous connection.
 }
 
 func TestServerShutdown(t *testing.T) { run(t, testServerShutdown) }