From bf380dec5e059ea6e7d07cec015dd0c913831a6a Mon Sep 17 00:00:00 2001
From: Arjan Singh Bal <46515553+arjan-bal@users.noreply.github.com>
Date: Wed, 22 Jan 2025 23:57:28 +0530
Subject: [PATCH] Cherrypick #7998,  #8011, #8010 into 1.70.x (#8028)

* server: fix buffer release timing in processUnaryRPC (#7998)

* xdsclient: release lock before attempting to close underlying transport (#8011)

* github: Run deps workflow against PR target branch and improve dir names (#8010)
---
 .github/workflows/deps.yml                  | 18 ++++++++++++------
 server.go                                   | 10 +++++++++-
 xds/internal/xdsclient/client_refcounted.go | 11 ++++++++---
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/.github/workflows/deps.yml b/.github/workflows/deps.yml
index c510e0e35fbb..17b05eb1cd93 100644
--- a/.github/workflows/deps.yml
+++ b/.github/workflows/deps.yml
@@ -30,14 +30,20 @@ jobs:
       # Run the commands to generate dependencies before and after and compare.
       - name: Compare dependencies
         run: |
-          BEFORE="$(mktemp -d)"
-          AFTER="$(mktemp -d)"
+          set -eu
+          TEMP_DIR="$(mktemp -d)"
+          # GITHUB_BASE_REF is set when the job is triggered by a PR.
+          TARGET_REF="${GITHUB_BASE_REF:-master}"
 
-          scripts/gen-deps.sh "${AFTER}"
-          git checkout origin/master
-          scripts/gen-deps.sh "${BEFORE}"
+          mkdir "${TEMP_DIR}/after"
+          scripts/gen-deps.sh "${TEMP_DIR}/after"
+
+          git checkout "origin/${TARGET_REF}"
+          mkdir "${TEMP_DIR}/before"
+          scripts/gen-deps.sh "${TEMP_DIR}/before"
 
           echo "Comparing dependencies..."
+          cd "${TEMP_DIR}"
           # Run grep in a sub-shell since bash does not support ! in the middle of a pipe
-          diff -u0 -r "${BEFORE}" "${AFTER}" | bash -c '! grep -v "@@"'
+          diff -u0 -r "before" "after" | bash -c '! grep -v "@@"'
           echo "No changes detected."
diff --git a/server.go b/server.go
index 16065a027ae8..9d5b2884d14e 100644
--- a/server.go
+++ b/server.go
@@ -1360,8 +1360,16 @@ func (s *Server) processUnaryRPC(ctx context.Context, stream *transport.ServerSt
 		}
 		return err
 	}
-	defer d.Free()
+	freed := false
+	dataFree := func() {
+		if !freed {
+			d.Free()
+			freed = true
+		}
+	}
+	defer dataFree()
 	df := func(v any) error {
+		defer dataFree()
 		if err := s.getCodec(stream.ContentSubtype()).Unmarshal(d, v); err != nil {
 			return status.Errorf(codes.Internal, "grpc: error unmarshalling request: %v", err)
 		}
diff --git a/xds/internal/xdsclient/client_refcounted.go b/xds/internal/xdsclient/client_refcounted.go
index 5a256c1bfada..f5fc76d8a75c 100644
--- a/xds/internal/xdsclient/client_refcounted.go
+++ b/xds/internal/xdsclient/client_refcounted.go
@@ -40,19 +40,24 @@ var (
 
 func clientRefCountedClose(name string) {
 	clientsMu.Lock()
-	defer clientsMu.Unlock()
-
 	client, ok := clients[name]
 	if !ok {
 		logger.Errorf("Attempt to close a non-existent xDS client with name %s", name)
+		clientsMu.Unlock()
 		return
 	}
 	if client.decrRef() != 0 {
+		clientsMu.Unlock()
 		return
 	}
+	delete(clients, name)
+	clientsMu.Unlock()
+
+	// This attempts to close the transport to the management server and could
+	// theoretically call back into the xdsclient package again and deadlock.
+	// Hence, this needs to be called without holding the lock.
 	client.clientImpl.close()
 	xdsClientImplCloseHook(name)
-	delete(clients, name)
 
 }