proto: fix race in Merge
Some existing targets (whether correctly or not) rely on it Merge
being safe to call concurrently so long as the set of fields being
merged are disjoint.
Change-Id: I4db9e64efccc7a2d44a5f9b52261b611cce461b0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196737
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/proto/merge.go b/proto/merge.go
index 45bfc9b..e1fb6b8 100644
--- a/proto/merge.go
+++ b/proto/merge.go
@@ -39,7 +39,9 @@
return true
})
- dst.SetUnknown(append(dst.GetUnknown(), src.GetUnknown()...))
+ if len(src.GetUnknown()) > 0 {
+ dst.SetUnknown(append(dst.GetUnknown(), src.GetUnknown()...))
+ }
}
func mergeList(dst, src protoreflect.List, fd protoreflect.FieldDescriptor) {
diff --git a/proto/merge_test.go b/proto/merge_test.go
index a282de7..2d25fb3 100644
--- a/proto/merge_test.go
+++ b/proto/merge_test.go
@@ -5,6 +5,7 @@
package proto_test
import (
+ "sync"
"testing"
"google.golang.org/protobuf/internal/encoding/pack"
@@ -368,3 +369,35 @@
})
}
}
+
+func TestMergeRace(t *testing.T) {
+ dst := new(testpb.TestAllTypes)
+ srcs := []*testpb.TestAllTypes{
+ {OptionalInt32: proto.Int32(1)},
+ {OptionalString: proto.String("hello")},
+ {RepeatedInt32: []int32{2, 3, 4}},
+ {RepeatedString: []string{"goodbye"}},
+ {MapStringString: map[string]string{"key": "value"}},
+ {OptionalNestedMessage: &testpb.TestAllTypes_NestedMessage{
+ A: proto.Int32(5),
+ }},
+ func() *testpb.TestAllTypes {
+ m := new(testpb.TestAllTypes)
+ m.ProtoReflect().SetUnknown(pack.Message{
+ pack.Tag{Number: 50000, Type: pack.VarintType}, pack.Svarint(-5),
+ }.Marshal())
+ return m
+ }(),
+ }
+
+ // It should be safe to concurrently merge non-overlapping fields.
+ var wg sync.WaitGroup
+ defer wg.Wait()
+ for _, src := range srcs {
+ wg.Add(1)
+ go func(src proto.Message) {
+ defer wg.Done()
+ proto.Merge(dst, src)
+ }(src)
+ }
+}
