Skip to content

Commit

Permalink
Cherrypick grpc#7998, grpc#8011, grpc#8010 into 1.70.x (grpc#8028)
Browse files Browse the repository at this point in the history
* server: fix buffer release timing in processUnaryRPC (grpc#7998)

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

* github: Run deps workflow against PR target branch and improve dir names (grpc#8010)
  • Loading branch information
arjan-bal authored Jan 22, 2025
1 parent 54b3eb9 commit bf380de
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 10 deletions.
18 changes: 12 additions & 6 deletions .github/workflows/deps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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."
10 changes: 9 additions & 1 deletion server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
11 changes: 8 additions & 3 deletions xds/internal/xdsclient/client_refcounted.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

}

Expand Down

0 comments on commit bf380de

Please sign in to comment.