Repository: grpc/grpc-go

Issue:
Race condition involving trailer metadata
### What version of gRPC are you using?

v1.72.0

### What version of Go are you using (`go version`)?

go version go1.24.2 linux/amd64

### What operating system (Linux, Windows, …) and version?

$ uname -a
Linux 6.8.0-1030-gcp #32~22.04.1-Ubuntu SMP Tue Apr 29 23:17:09 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux

### What did you do?

We observed the following panic in grpc code.

```
fatal error: concurrent map iteration and map write

goroutine 13860950 [running]:
internal/runtime/maps.fatal({0x28de412?, 0x45df57?})
        /usr/local/go/src/runtime/panic.go:1058 +0x18
internal/runtime/maps.(*Iter).Next(0x224e100?)
        /usr/local/go/src/internal/runtime/maps/table.go:683 +0x86
google.golang.org/grpc/metadata.MD.Copy(...)
        /go/src/github.com/lightstep/vendor/google.golang.org/grpc/metadata/metadata.go:101
google.golang.org/grpc/internal/transport.(*serverHandlerTransport).writeStatus(0xc00a544ee0, 0xc0274c3320, 0xc016ef6388)
        /go/src/github.com/lightstep/vendor/google.golang.org/grpc/internal/transport/handler_server.go:282 +0x3c5
google.golang.org/grpc/internal/transport.(*ServerStream).WriteStatus(...)
        /go/src/github.com/lightstep/vendor/google.golang.org/grpc/internal/transport/server_stream.go:76
google.golang.org/grpc.(*Server).processUnaryRPC(0xc000a63200, {0x2c782f0, 0xc02152a6f0}, 0xc0274c3320, 0xc001ac2cc0, 0x43e0550, 0x0)
        /go/src/github.com/lightstep/vendor/google.golang.org/grpc/server.go:1418 +0x1562
google.golang.org/grpc.(*Server).handleStream(0xc000a63200, {0x2c794a8, 0xc00a544ee0}, 0xc0274c3320)
        /go/src/github.com/lightstep/vendor/google.golang.org/grpc/server.go:1815 +0xb88
google.golang.org/grpc.(*Server).serveStreams.func2.1()
        /go/src/github.com/lightstep/vendor/google.golang.org/grpc/server.go:1035 +0x7f
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 13860947
        /go/src/github.com/lightstep/vendor/google.golang.org/grpc/server.go:1046 +0x11d
```

I suspect the problem may be a race internal to grpc code, because this mutex [1] does not appear to be held at the callsite [2] where the panic occurred, despite a comment suggesting that it should be ("hdrMu protects outgoing header and trailer metadata").

(It's also possible that our application code is touching the trailer metadata in some way that races with the call stack above but I've been unable to find such a race, and I stopped looking hard after finding this potential issue with the unheld mutex.)

[1] https://github.com/grpc/grpc-go/blob/4375c784450aa7e43ff15b8b2879c896d0917130/internal/transport/server_stream.go#L49

[2] https://github.com/grpc/grpc-go/blob/4375c784450aa7e43ff15b8b2879c896d0917130/internal/transport/handler_server.go#L282


Discussion:
@isaacnorman82: Also seeing on the same version and very similar callstack.@arjan-bal: Hi @akehlenbeck, thanks for filing the issue. I agree with the root cause analysis, the mutex should be held while  copying the trailers. We do lock the mutex in [the regular gRPC server transport](https://github.com/grpc/grpc-go/blob/9ac0ec87ca2ecc66b3c0c084708aef768637aef6/internal/transport/http2_server.go#L1140-L1142), but have missed it in the std lib http/2 transport.

I'm trying to repro the race in a unit test to ensure we can catch regressions in the future. Will raise a PR once I have the repro.@arjan-bal: The only place where a write happens is `writeStatus` is when the status contains a proto. I'm able to get the race detector to catch this.

https://github.com/grpc/grpc-go/blob/4375c784450aa7e43ff15b8b2879c896d0917130/internal/transport/handler_server.go#L251-L252@isaacnorman82: Thanks @arjan-bal, what's the normal time from a bug fix going in to it being in a formal release?@arjan-bal: Hi @isaacnorman82, this doesn't appear to be a recent regression, so it will be included in the next minor release. Version 1.75 is scheduled for August 19th, but it may be delayed while we wait for some PRs to be merged. For recent regressions, we perform patch releases for the affected versions as soon as possible.

https://github.com/grpc/grpc-go/milestones @isaacnorman82: This is causing frequent crashes in prod for us.  I don't believe there's a way to mitigate. 

We can fork with your fix until the next release, but I do think this bug is more severe than you suggest. 

Is there anything from our code we can do to avoid this?@arjan-bal: I should be able to merge the change into master and the [1.75 branch](https://github.com/grpc/grpc-go/tree/v1.75.x) in 1-2 days. In my opinion, using a commit from a grpc-go release branch would be safer than one from my fork. The 1.75 release should happen in the next 1-2 weeks.

> Is there anything from our code we can do to avoid this?

What caused the issue to start occurring? Maybe you could rollback those dependency updates to fix things?@isaacnorman82: Yeah thanks, I've been watching the PR and once that's agreed and merged I'll take that commit.

We're not sure exactly what change caused this to start occurring for us, I have a suspicion based on the nature of the bug that it's load related.  It's seen quite far back in our logs and it's too intermittent to easily reproduce in our full stack.

Solution patch:
diff --git a/internal/transport/handler_server.go b/internal/transport/handler_server.go
index 3dea2357..d954a64c 100644
--- a/internal/transport/handler_server.go
+++ b/internal/transport/handler_server.go
@@ -277,11 +277,13 @@ func (ht *serverHandlerTransport) writeStatus(s *ServerStream, st *status.Status
 	if err == nil { // transport has not been closed
 		// Note: The trailer fields are compressed with hpack after this call returns.
 		// No WireLength field is set here.
+		s.hdrMu.Lock()
 		for _, sh := range ht.stats {
 			sh.HandleRPC(s.Context(), &stats.OutTrailer{
 				Trailer: s.trailer.Copy(),
 			})
 		}
+		s.hdrMu.Unlock()
 	}
 	ht.Close(errors.New("finished writing status"))
 	return err



Test patch:
diff --git a/internal/transport/handler_server_test.go b/internal/transport/handler_server_test.go
index 91102283..e64af274 100644
--- a/internal/transport/handler_server_test.go
+++ b/internal/transport/handler_server_test.go
@@ -35,6 +35,7 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/mem"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/stats"
 	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/protoadapt"
@@ -246,7 +247,26 @@ type handleStreamTest struct {
 	ht    *serverHandlerTransport
 }
 
-func newHandleStreamTest(t *testing.T) *handleStreamTest {
+type mockStatsHandler struct {
+	rpcStatsCh chan stats.RPCStats
+}
+
+func (h *mockStatsHandler) TagRPC(ctx context.Context, _ *stats.RPCTagInfo) context.Context {
+	return ctx
+}
+
+func (h *mockStatsHandler) HandleRPC(_ context.Context, s stats.RPCStats) {
+	h.rpcStatsCh <- s
+}
+
+func (h *mockStatsHandler) TagConn(ctx context.Context, _ *stats.ConnTagInfo) context.Context {
+	return ctx
+}
+
+func (h *mockStatsHandler) HandleConn(context.Context, stats.ConnStats) {
+}
+
+func newHandleStreamTest(t *testing.T, statsHandlers []stats.Handler) *handleStreamTest {
 	bodyr, bodyw := io.Pipe()
 	req := &http.Request{
 		ProtoMajor: 2,
@@ -260,7 +280,7 @@ func newHandleStreamTest(t *testing.T) *handleStreamTest {
 		Body: bodyr,
 	}
 	rw := newTestHandlerResponseWriter().(testHandlerResponseWriter)
-	ht, err := NewServerHandlerTransport(rw, req, nil, mem.DefaultBufferPool())
+	ht, err := NewServerHandlerTransport(rw, req, statsHandlers, mem.DefaultBufferPool())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -273,7 +293,7 @@ func newHandleStreamTest(t *testing.T) *handleStreamTest {
 }
 
 func (s) TestHandlerTransport_HandleStreams(t *testing.T) {
-	st := newHandleStreamTest(t)
+	st := newHandleStreamTest(t, nil)
 	handleStream := func(s *ServerStream) {
 		if want := "/service/foo.bar"; s.method != want {
 			t.Errorf("stream method = %q; want %q", s.method, want)
@@ -342,7 +362,7 @@ func (s) TestHandlerTransport_HandleStreams_InvalidArgument(t *testing.T) {
 }
 
 func handleStreamCloseBodyTest(t *testing.T, statusCode codes.Code, msg string) {
-	st := newHandleStreamTest(t)
+	st := newHandleStreamTest(t, nil)
 
 	handleStream := func(s *ServerStream) {
 		s.WriteStatus(status.New(statusCode, msg))
@@ -451,7 +471,7 @@ func (s) TestHandlerTransport_HandleStreams_WriteStatusWrite(t *testing.T) {
 }
 
 func testHandlerTransportHandleStreams(t *testing.T, handleStream func(st *handleStreamTest, s *ServerStream)) {
-	st := newHandleStreamTest(t)
+	st := newHandleStreamTest(t, nil)
 	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
 	t.Cleanup(cancel)
 	st.ht.HandleStreams(
@@ -483,7 +503,7 @@ func (s) TestHandlerTransport_HandleStreams_ErrDetails(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	hst := newHandleStreamTest(t)
+	hst := newHandleStreamTest(t, nil)
 	handleStream := func(s *ServerStream) {
 		s.WriteStatus(st)
 	}
@@ -506,11 +526,81 @@ func (s) TestHandlerTransport_HandleStreams_ErrDetails(t *testing.T) {
 	checkHeaderAndTrailer(t, hst.rw, wantHeader, wantTrailer)
 }
 
+// Tests the use of stats handlers and ensures there are no data races while
+// accessing trailers.
+func (s) TestHandlerTransport_HandleStreams_StatsHandlers(t *testing.T) {
+	errDetails := []protoadapt.MessageV1{
+		&epb.RetryInfo{
+			RetryDelay: &durationpb.Duration{Seconds: 60},
+		},
+		&epb.ResourceInfo{
+			ResourceType: "foo bar",
+			ResourceName: "service.foo.bar",
+			Owner:        "User",
+		},
+	}
+
+	statusCode := codes.ResourceExhausted
+	msg := "you are being throttled"
+	st, err := status.New(statusCode, msg).WithDetails(errDetails...)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	stBytes, err := proto.Marshal(st.Proto())
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Add mock stats handlers to exercise the stats handler code path.
+	statsHandler := &mockStatsHandler{
+		rpcStatsCh: make(chan stats.RPCStats, 2),
+	}
+	hst := newHandleStreamTest(t, []stats.Handler{statsHandler})
+	handleStream := func(s *ServerStream) {
+		if err := s.SendHeader(metadata.New(map[string]string{})); err != nil {
+			t.Error(err)
+		}
+		if err := s.SetTrailer(metadata.Pairs("custom-trailer", "Custom trailer value")); err != nil {
+			t.Error(err)
+		}
+		s.WriteStatus(st)
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+	hst.ht.HandleStreams(
+		ctx, func(s *ServerStream) { go handleStream(s) },
+	)
+	wantHeader := http.Header{
+		"Date":         nil,
+		"Content-Type": {"application/grpc"},
+		"Trailer":      {"Grpc-Status", "Grpc-Message", "Grpc-Status-Details-Bin"},
+	}
+	wantTrailer := http.Header{
+		"Grpc-Status":             {fmt.Sprint(uint32(statusCode))},
+		"Grpc-Message":            {encodeGrpcMessage(msg)},
+		"Grpc-Status-Details-Bin": {encodeBinHeader(stBytes)},
+		"Custom-Trailer":          []string{"Custom trailer value"},
+	}
+
+	checkHeaderAndTrailer(t, hst.rw, wantHeader, wantTrailer)
+	wantStatTypes := []stats.RPCStats{&stats.OutHeader{}, &stats.OutTrailer{}}
+	for _, wantType := range wantStatTypes {
+		select {
+		case <-ctx.Done():
+			t.Fatal("Context timed out waiting for statsHandler.HandleRPC() to be called.")
+		case s := <-statsHandler.rpcStatsCh:
+			if reflect.TypeOf(s) != reflect.TypeOf(wantType) {
+				t.Fatalf("Received RPCStats of type %T, want %T", s, wantType)
+			}
+		}
+	}
+}
+
 // TestHandlerTransport_Drain verifies that Drain() is not implemented
 // by `serverHandlerTransport`.
 func (s) TestHandlerTransport_Drain(t *testing.T) {
 	defer func() { recover() }()
-	st := newHandleStreamTest(t)
+	st := newHandleStreamTest(t, nil)
 	st.ht.Drain("whatever")
 	t.Errorf("serverHandlerTransport.Drain() should have panicked")
 }