From cf5217617cd4fe081feb40349138c5c90b7be913 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Mon, 21 Feb 2022 17:52:39 +0000 Subject: [PATCH] backchannel: do not block when closing client In https://gitlab.com/gitlab-org/gitlab/-/issues/351425 we learned that when grpc-go reconnects to a Gitaly server, it synchronously closes the old connection. That makes it important that we do not block while closing a connection. This commit removes code that waited for the backchannel client-side server to close while closing a client connection. Instead of waiting, we just propagate the close to the yamux sesion. Changelog: fixed --- internal/backchannel/client.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/internal/backchannel/client.go b/internal/backchannel/client.go index fd252d349f6..c3512f8a616 100644 --- a/internal/backchannel/client.go +++ b/internal/backchannel/client.go @@ -111,21 +111,14 @@ func (ch clientHandshake) serve(ctx context.Context, conn net.Conn) (net.Conn, e // Run the backchannel server. server := ch.serverFactory() - serveErr := make(chan error, 1) - go func() { serveErr <- server.Serve(muxSession) }() + go func() { server.Serve(muxSession) }() return connCloser{ Conn: clientToServer, close: func() error { - // Stop closes the listener, which is the muxing session. Closing the - // muxing session closes the underlying network connection. - // - // There's no sense in doing a graceful shutdown. The connection is being closed, - // it would no longer receive a response from the server. - server.Stop() - // Serve returns a non-nil error if it returned before Stop was called. If the error - // is non-nil, it indicates a serving failure prior to calling Stop. - return <-serveErr + // When the clientToServer yamux stream gets closed for whatever reason, + // we want to close the whole yamux session along with it. + return muxSession.Close() }, }, nil } -- GitLab