Skip to content

Commit bf380de

Browse files
authored
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)
1 parent 54b3eb9 commit bf380de

File tree

3 files changed

+29
-10
lines changed

3 files changed

+29
-10
lines changed

.github/workflows/deps.yml

+12-6
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,20 @@ jobs:
3030
# Run the commands to generate dependencies before and after and compare.
3131
- name: Compare dependencies
3232
run: |
33-
BEFORE="$(mktemp -d)"
34-
AFTER="$(mktemp -d)"
33+
set -eu
34+
TEMP_DIR="$(mktemp -d)"
35+
# GITHUB_BASE_REF is set when the job is triggered by a PR.
36+
TARGET_REF="${GITHUB_BASE_REF:-master}"
3537
36-
scripts/gen-deps.sh "${AFTER}"
37-
git checkout origin/master
38-
scripts/gen-deps.sh "${BEFORE}"
38+
mkdir "${TEMP_DIR}/after"
39+
scripts/gen-deps.sh "${TEMP_DIR}/after"
40+
41+
git checkout "origin/${TARGET_REF}"
42+
mkdir "${TEMP_DIR}/before"
43+
scripts/gen-deps.sh "${TEMP_DIR}/before"
3944
4045
echo "Comparing dependencies..."
46+
cd "${TEMP_DIR}"
4147
# Run grep in a sub-shell since bash does not support ! in the middle of a pipe
42-
diff -u0 -r "${BEFORE}" "${AFTER}" | bash -c '! grep -v "@@"'
48+
diff -u0 -r "before" "after" | bash -c '! grep -v "@@"'
4349
echo "No changes detected."

server.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -1360,8 +1360,16 @@ func (s *Server) processUnaryRPC(ctx context.Context, stream *transport.ServerSt
13601360
}
13611361
return err
13621362
}
1363-
defer d.Free()
1363+
freed := false
1364+
dataFree := func() {
1365+
if !freed {
1366+
d.Free()
1367+
freed = true
1368+
}
1369+
}
1370+
defer dataFree()
13641371
df := func(v any) error {
1372+
defer dataFree()
13651373
if err := s.getCodec(stream.ContentSubtype()).Unmarshal(d, v); err != nil {
13661374
return status.Errorf(codes.Internal, "grpc: error unmarshalling request: %v", err)
13671375
}

xds/internal/xdsclient/client_refcounted.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,24 @@ var (
4040

4141
func clientRefCountedClose(name string) {
4242
clientsMu.Lock()
43-
defer clientsMu.Unlock()
44-
4543
client, ok := clients[name]
4644
if !ok {
4745
logger.Errorf("Attempt to close a non-existent xDS client with name %s", name)
46+
clientsMu.Unlock()
4847
return
4948
}
5049
if client.decrRef() != 0 {
50+
clientsMu.Unlock()
5151
return
5252
}
53+
delete(clients, name)
54+
clientsMu.Unlock()
55+
56+
// This attempts to close the transport to the management server and could
57+
// theoretically call back into the xdsclient package again and deadlock.
58+
// Hence, this needs to be called without holding the lock.
5359
client.clientImpl.close()
5460
xdsClientImplCloseHook(name)
55-
delete(clients, name)
5661

5762
}
5863

0 commit comments

Comments
 (0)