server: close net.Conn unconditionally after accepting

Informs #105448

Previously after the server accepted a connection, it was closed in multiple
paths and in multiple layers after it was done being used. Now it is always
closed in the same layer that accepted it after the serveConn callback returns.

Release note: None

pkg/server/server_controller_sql.go

@@ -50,6 +50,10 @@ func (c *serverController) sqlMux(
 			// The concurrent dispatch gives a chance to the succeeding
 			// servers to see and process the cancel at approximately the
 			// same time as every other.
+			//
+			// Cancel requests are unauthenticated so run the cancel async to prevent
+			// the client from deriving any info about the cancel based on how long it
+			// takes.
 			if err := c.stopper.RunAsyncTask(ctx, "cancel", func(ctx context.Context) {
 				s.handleCancel(ctx, status.CancelKey)
 			}); err != nil {

pkg/server/tenant.go

@@ -833,8 +833,12 @@ func (s *SQLServerWrapper) serveConn(
 	pgServer := s.PGServer()
 	switch status.State {
 	case pgwire.PreServeCancel:
-		pgServer.HandleCancel(ctx, status.CancelKey)
-		return nil
+		// Cancel requests are unauthenticated so run the cancel async to prevent
+		// the client from deriving any info about the cancel based on how long it
+		// takes.
+		return s.stopper.RunAsyncTask(ctx, "cancel", func(ctx context.Context) {
+			pgServer.HandleCancel(ctx, status.CancelKey)
+		})
 	case pgwire.PreServeReady:
 		return pgServer.ServeConn(ctx, conn, status)
 	default:

pkg/sql/pgwire/pre_serve.go

@@ -217,7 +217,6 @@ func (s *PreServeConnHandler) SendRoutingError(
 		`Double check your "-ccluster=" connection option or your "cluster:" database name prefix.`)
 
 	_ = s.sendErr(ctx, s.st, conn, err)
-	_ = conn.Close()
 }
 
 // sendErr sends errors to the client during the connection startup
@@ -235,7 +234,6 @@ func (s *PreServeConnHandler) sendErr(
 	// receive error payload are highly correlated with clients
 	// disconnecting abruptly.
 	_ /* err */ = w.writeErr(ctx, err, conn)
-	_ = conn.Close()
 	return err
 }
 
@@ -324,7 +322,7 @@ func (s *PreServeConnHandler) PreServe(
 	case versionCancel:
 		// The cancel message is rather peculiar: it is sent without
 		// authentication, always over an unencrypted channel.
-		if ok, key := readCancelKeyAndCloseConn(ctx, conn, &buf); ok {
+		if ok, key := readCancelKey(ctx, &buf); ok {
 			return conn, PreServeStatus{
 				State:     PreServeCancel,
 				CancelKey: key,
@@ -374,7 +372,7 @@ func (s *PreServeConnHandler) PreServe(
 		// Yet, we've found clients in the wild that send the cancel
 		// after the TLS handshake, for example at
 		// https://github.com/cockroachlabs/support/issues/600.
-		if ok, key := readCancelKeyAndCloseConn(ctx, conn, &buf); ok {
+		if ok, key := readCancelKey(ctx, &buf); ok {
 			return conn, PreServeStatus{
 				State:     PreServeCancel,
 				CancelKey: key,

pkg/sql/pgwire/server.go

@@ -928,8 +928,6 @@ func (s *Server) serveImpl(
 	authOpt authOptions,
 	sessionID clusterunique.ID,
 ) {
-	defer func() { _ = c.conn.Close() }()
-
 	if c.sessionArgs.User.IsRootUser() || c.sessionArgs.User.IsNodeUser() {
 		ctx = logtags.AddTag(ctx, "user", redact.Safe(c.sessionArgs.User))
 	} else {
@@ -1253,18 +1251,13 @@ func (s *Server) serveImpl(
 	}
 }
 
-// readCancelKeyAndCloseConn retrieves the "backend data" key that identifies
+// readCancelKey retrieves the "backend data" key that identifies
 // a cancellable query, then closes the connection.
-func readCancelKeyAndCloseConn(
-	ctx context.Context, conn net.Conn, buf *pgwirebase.ReadBuffer,
+func readCancelKey(
+	ctx context.Context, buf *pgwirebase.ReadBuffer,
 ) (ok bool, cancelKey pgwirecancel.BackendKeyData) {
 	telemetry.Inc(sqltelemetry.CancelRequestCounter)
 	backendKeyDataBits, err := buf.GetUint64()
-	// The connection that issued the cancel is not a SQL session -- it's an
-	// entirely new connection that's created just to send the cancel. We close
-	// the connection as soon as possible after reading the data, since there
-	// is nothing to send back to the client.
-	_ = conn.Close()
 	// The client is also unwilling to read an error payload, so we just log it locally.
 	if err != nil {
 		log.Sessions.Warningf(ctx, "%v", errors.Wrap(err, "reading cancel key from client"))
@@ -1378,7 +1371,6 @@ func (s *Server) sendErr(
 	// receive error payload are highly correlated with clients
 	// disconnecting abruptly.
 	_ /* err */ = w.writeErr(ctx, err, conn)
-	_ = conn.Close()
 	return err
 }
 

pkg/util/netutil/net.go

@@ -183,6 +183,9 @@ func (s *TCPServer) ServeWith(
 		}
 		tempDelay = 0
 		err := s.stopper.RunAsyncTask(ctx, "tcp-serve", func(ctx context.Context) {
+			defer func() {
+				_ = rw.Close()
+			}()
 			s.addConn(rw)
 			defer s.rmConn(rw)
 			serveConn(ctx, rw)