Merge #83335

83335: sql/pgwire: fix a leak of accounted for memory r=rafiss a=joshimhoff

`@rafiss,` can I get a suggestion on where to add an automated test?

Fixes #83034.

**sql/pgwire: fix a leak of accounted for memory**

This commit fixes a bug where in case client-provided session params failed
to parse, CRDB leaked accounted for memory.

Release note (bug fix): In case client-provided session params fail to parse,
CRDB no longer leaks accounted for memory.

Co-authored-by: Josh Imhoff <[email protected]>

pkg/sql/pgwire/conn_test.go

@@ -1937,3 +1937,33 @@ func TestPGWireRejectsNewConnIfTooManyConns(t *testing.T) {
 		requireConnectionCount(t, 0)
 	})
 }
+
+func TestConnCloseReleasesReservedMem(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+	s, _, _ := serverutils.StartServer(t, base.TestServerArgs{})
+	ctx := context.Background()
+	defer s.Stopper().Stop(ctx)
+
+	before := s.PGServer().(*Server).connMonitor.AllocBytes()
+
+	pgURL, cleanupFunc := sqlutils.PGUrl(
+		t, s.ServingSQLAddr(), "testConnClose" /* prefix */, url.User(username.RootUser),
+	)
+	values := pgURL.Query()
+	values.Add("options", "c sadsad=") // invalid client-provided session param
+	pgURL.RawQuery = values.Encode()
+
+	defer cleanupFunc()
+	db, err := gosql.Open("postgres", pgURL.String())
+	require.NoError(t, err)
+	defer db.Close()
+
+	err = db.Ping()
+	require.Error(t, err)
+	require.Regexp(t, "pq: option .* is invalid", err.Error())
+
+	// Check that no accounted-for memory is leaked, after the connection attempt fails.
+	after := s.PGServer().(*Server).connMonitor.AllocBytes()
+	require.Equal(t, before, after)
+}

pkg/sql/pgwire/server.go

@@ -850,6 +850,7 @@ func (s *Server) ServeConn(ctx context.Context, conn net.Conn, socketType Socket
 	var sArgs sql.SessionArgs
 	if sArgs, err = parseClientProvidedSessionParameters(ctx, &s.execCfg.Settings.SV, &buf,
 		conn.RemoteAddr(), s.trustClientProvidedRemoteAddr.Get()); err != nil {
+		reserved.Close(ctx)
 		return s.sendErr(ctx, conn, err)
 	}