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>

proto/merge.go

@@ -39,7 +39,9 @@ func mergeMessage(dst, src protoreflect.Message) {
 		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) {

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 TestMerge(t *testing.T) {
 		})
 	}
 }
+
+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)
+	}
+}