Cherrypick #7998, #8011, #8010 into 1.70.x (#8028) · grpc/grpc-go@bf380de (original) (raw)
3 files changed
lines changed
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -30,14 +30,20 @@ jobs: | ||
30 | 30 | # Run the commands to generate dependencies before and after and compare. |
31 | 31 | - name: Compare dependencies |
32 | 32 | 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}" | |
35 | 37 | |
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" | |
39 | 44 | |
40 | 45 | echo "Comparing dependencies..." |
46 | + cd "${TEMP_DIR}" | |
41 | 47 | # 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 "@@"' | |
43 | 49 | echo "No changes detected." |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1360,8 +1360,16 @@ func (s *Server) processUnaryRPC(ctx context.Context, stream *transport.ServerSt | ||
1360 | 1360 | } |
1361 | 1361 | return err |
1362 | 1362 | } |
1363 | -defer d.Free() | |
1363 | +freed := false | |
1364 | +dataFree := func() { | |
1365 | +if !freed { | |
1366 | +d.Free() | |
1367 | +freed = true | |
1368 | + } | |
1369 | + } | |
1370 | +defer dataFree() | |
1364 | 1371 | df := func(v any) error { |
1372 | +defer dataFree() | |
1365 | 1373 | if err := s.getCodec(stream.ContentSubtype()).Unmarshal(d, v); err != nil { |
1366 | 1374 | return status.Errorf(codes.Internal, "grpc: error unmarshalling request: %v", err) |
1367 | 1375 | } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -40,19 +40,24 @@ var ( | ||
40 | 40 | |
41 | 41 | func clientRefCountedClose(name string) { |
42 | 42 | clientsMu.Lock() |
43 | -defer clientsMu.Unlock() | |
44 | - | |
45 | 43 | client, ok := clients[name] |
46 | 44 | if !ok { |
47 | 45 | logger.Errorf("Attempt to close a non-existent xDS client with name %s", name) |
46 | +clientsMu.Unlock() | |
48 | 47 | return |
49 | 48 | } |
50 | 49 | if client.decrRef() != 0 { |
50 | +clientsMu.Unlock() | |
51 | 51 | return |
52 | 52 | } |
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. | |
53 | 59 | client.clientImpl.close() |
54 | 60 | xdsClientImplCloseHook(name) |
55 | -delete(clients, name) | |
56 | 61 | |
57 | 62 | } |
58 | 63 |