Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions lib/kube/proxy/forwarder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2426,6 +2426,11 @@ func TestGOAWAYHandling_Concurrent(t *testing.T) {
type goawayServer struct {
listener net.Listener
tlsConfig *tls.Config

mu sync.Mutex
closed bool
conns []net.Conn
wg sync.WaitGroup
}

// URL returns the address clients should use to connect to the server.
Expand All @@ -2447,15 +2452,31 @@ func (g *goawayServer) Serve() error {
return err
}

if err := g.handleConn(conn); err != nil {
return err
g.mu.Lock()
if g.closed {
g.mu.Unlock()
_ = conn.Close()
continue
}
g.conns = append(g.conns, conn)
g.wg.Go(func() { _ = g.handleConn(conn) })
g.mu.Unlock()
}
}

// Close terminates the server and unblocks any calls to [Serve].
// Close terminates the server and unblocks any calls to [Serve], then
// closes all in-flight connections and waits for their handler goroutines
// to exit.
func (g *goawayServer) Close() error {
return g.listener.Close()
err := g.listener.Close()
g.mu.Lock()
g.closed = true
for _, c := range g.conns {
_ = c.Close()
}
g.mu.Unlock()
g.wg.Wait()
Comment on lines +2477 to +2478
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent Close from returning before accepted conn handlers start

A connection accepted just before Close() takes g.mu can be appended and launched after the close loop unlocks, because Serve() does append+wg.Go under the same mutex but Close() releases the mutex before wg.Wait(). In that interleaving, Close() can miss closing that conn and return without waiting for its handler, leaving a goroutine blocked in TLS/frame reads and reintroducing teardown flakiness under concurrent tests.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor but I addressed in 8e73d4d

return err
}

// handleConn performs the initial HTTP/2 message exchange and then
Expand Down
Loading