internal/filedesc: make descriptor initialization goroutine-safe

Before this change there was a data race if you initialized the
descriptor of a message in parallel to the descriptor of any extension of
that message because the extendees feature set is read during the
initialization of the extension.

Change-Id: Id896b9fbf848209fce7dae8a7f40e2d61a3b2825
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/568695
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Stapelberg <stapelberg@google.com>
This commit is contained in:
Lasse Folger 2024-03-04 09:16:51 +01:00
parent 6bec1ef16e
commit fe89159266
7 changed files with 402 additions and 8 deletions

View File

@ -319,6 +319,7 @@ func (md *Message) unmarshalSeed(b []byte, sb *strs.Builder, pf *File, pd protor
md.L0.ParentFile = pf
md.L0.Parent = pd
md.L0.Index = i
md.L1.EditionFeatures = featuresFromParentDesc(md.Parent())
var prevField protoreflect.FieldNumber
var numEnums, numMessages, numExtensions int
@ -424,6 +425,13 @@ func (md *Message) unmarshalSeedOptions(b []byte) {
case genid.MessageOptions_MessageSetWireFormat_field_number:
md.L1.IsMessageSet = protowire.DecodeBool(v)
}
case protowire.BytesType:
v, m := protowire.ConsumeBytes(b)
b = b[m:]
switch num {
case genid.MessageOptions_Features_field_number:
md.L1.EditionFeatures = unmarshalFeatureSet(v, md.L1.EditionFeatures)
}
default:
m := protowire.ConsumeFieldValue(num, typ, b)
b = b[m:]

View File

@ -280,7 +280,6 @@ func (md *Message) unmarshalFull(b []byte, sb *strs.Builder) {
var rawFields, rawOneofs [][]byte
var enumIdx, messageIdx, extensionIdx int
var rawOptions []byte
md.L1.EditionFeatures = featuresFromParentDesc(md.Parent())
md.L2 = new(MessageL2)
for len(b) > 0 {
num, typ, n := protowire.ConsumeTag(b)
@ -353,13 +352,6 @@ func (md *Message) unmarshalOptions(b []byte) {
case genid.MessageOptions_MessageSetWireFormat_field_number:
md.L1.IsMessageSet = protowire.DecodeBool(v)
}
case protowire.BytesType:
v, m := protowire.ConsumeBytes(b)
b = b[m:]
switch num {
case genid.MessageOptions_Features_field_number:
md.L1.EditionFeatures = unmarshalFeatureSet(v, md.L1.EditionFeatures)
}
default:
m := protowire.ConsumeFieldValue(num, typ, b)
b = b[m:]

View File

@ -0,0 +1,168 @@
// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// Code generated by protoc-gen-go. DO NOT EDIT.
// source: internal/testprotos/race/extender/test.proto
package extender
import (
message "google.golang.org/protobuf/internal/testprotos/race/message"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
reflect "reflect"
sync "sync"
)
type OtherMessage struct {
state protoimpl.MessageState
sizeCache protoimpl.SizeCache
unknownFields protoimpl.UnknownFields
I32 *int32 `protobuf:"varint,1,opt,name=i32" json:"i32,omitempty"`
}
func (x *OtherMessage) Reset() {
*x = OtherMessage{}
if protoimpl.UnsafeEnabled {
mi := &file_internal_testprotos_race_extender_test_proto_msgTypes[0]
ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x))
ms.StoreMessageInfo(mi)
}
}
func (x *OtherMessage) String() string {
return protoimpl.X.MessageStringOf(x)
}
func (*OtherMessage) ProtoMessage() {}
func (x *OtherMessage) ProtoReflect() protoreflect.Message {
mi := &file_internal_testprotos_race_extender_test_proto_msgTypes[0]
if protoimpl.UnsafeEnabled && x != nil {
ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x))
if ms.LoadMessageInfo() == nil {
ms.StoreMessageInfo(mi)
}
return ms
}
return mi.MessageOf(x)
}
// Deprecated: Use OtherMessage.ProtoReflect.Descriptor instead.
func (*OtherMessage) Descriptor() ([]byte, []int) {
return file_internal_testprotos_race_extender_test_proto_rawDescGZIP(), []int{0}
}
func (x *OtherMessage) GetI32() int32 {
if x != nil && x.I32 != nil {
return *x.I32
}
return 0
}
var file_internal_testprotos_race_extender_test_proto_extTypes = []protoimpl.ExtensionInfo{
{
ExtendedType: (*message.MyMessage)(nil),
ExtensionType: (*string)(nil),
Field: 2,
Name: "goproto.proto.test.s",
Tag: "bytes,2,opt,name=s",
Filename: "internal/testprotos/race/extender/test.proto",
},
}
// Extension fields to message.MyMessage.
var (
// optional string s = 2;
E_S = &file_internal_testprotos_race_extender_test_proto_extTypes[0]
)
var File_internal_testprotos_race_extender_test_proto protoreflect.FileDescriptor
var file_internal_testprotos_race_extender_test_proto_rawDesc = []byte{
0x0a, 0x2c, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x74, 0x65, 0x73, 0x74, 0x70,
0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2f, 0x72, 0x61, 0x63, 0x65, 0x2f, 0x65, 0x78, 0x74, 0x65, 0x6e,
0x64, 0x65, 0x72, 0x2f, 0x74, 0x65, 0x73, 0x74, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x12,
0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x74, 0x65,
0x73, 0x74, 0x1a, 0x2b, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x74, 0x65, 0x73,
0x74, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2f, 0x72, 0x61, 0x63, 0x65, 0x2f, 0x6d, 0x65, 0x73,
0x73, 0x61, 0x67, 0x65, 0x2f, 0x74, 0x65, 0x73, 0x74, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22,
0x20, 0x0a, 0x0c, 0x4f, 0x74, 0x68, 0x65, 0x72, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12,
0x10, 0x0a, 0x03, 0x69, 0x33, 0x32, 0x18, 0x01, 0x20, 0x01, 0x28, 0x05, 0x52, 0x03, 0x69, 0x33,
0x32, 0x3a, 0x2b, 0x0a, 0x01, 0x73, 0x12, 0x1d, 0x2e, 0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f,
0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x74, 0x65, 0x73, 0x74, 0x2e, 0x4d, 0x79, 0x4d, 0x65,
0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x01, 0x73, 0x42, 0x3e,
0x5a, 0x3c, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x67, 0x6f, 0x6c, 0x61, 0x6e, 0x67, 0x2e,
0x6f, 0x72, 0x67, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2f, 0x69, 0x6e, 0x74,
0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x74, 0x65, 0x73, 0x74, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73,
0x2f, 0x72, 0x61, 0x63, 0x65, 0x2f, 0x65, 0x78, 0x74, 0x65, 0x6e, 0x64, 0x65, 0x72, 0x62, 0x08,
0x65, 0x64, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x70, 0xe8, 0x07,
}
var (
file_internal_testprotos_race_extender_test_proto_rawDescOnce sync.Once
file_internal_testprotos_race_extender_test_proto_rawDescData = file_internal_testprotos_race_extender_test_proto_rawDesc
)
func file_internal_testprotos_race_extender_test_proto_rawDescGZIP() []byte {
file_internal_testprotos_race_extender_test_proto_rawDescOnce.Do(func() {
file_internal_testprotos_race_extender_test_proto_rawDescData = protoimpl.X.CompressGZIP(file_internal_testprotos_race_extender_test_proto_rawDescData)
})
return file_internal_testprotos_race_extender_test_proto_rawDescData
}
var file_internal_testprotos_race_extender_test_proto_msgTypes = make([]protoimpl.MessageInfo, 1)
var file_internal_testprotos_race_extender_test_proto_goTypes = []interface{}{
(*OtherMessage)(nil), // 0: goproto.proto.test.OtherMessage
(*message.MyMessage)(nil), // 1: goproto.proto.test.MyMessage
}
var file_internal_testprotos_race_extender_test_proto_depIdxs = []int32{
1, // 0: goproto.proto.test.s:extendee -> goproto.proto.test.MyMessage
1, // [1:1] is the sub-list for method output_type
1, // [1:1] is the sub-list for method input_type
1, // [1:1] is the sub-list for extension type_name
0, // [0:1] is the sub-list for extension extendee
0, // [0:0] is the sub-list for field type_name
}
func init() { file_internal_testprotos_race_extender_test_proto_init() }
func file_internal_testprotos_race_extender_test_proto_init() {
if File_internal_testprotos_race_extender_test_proto != nil {
return
}
if !protoimpl.UnsafeEnabled {
file_internal_testprotos_race_extender_test_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} {
switch v := v.(*OtherMessage); i {
case 0:
return &v.state
case 1:
return &v.sizeCache
case 2:
return &v.unknownFields
default:
return nil
}
}
}
type x struct{}
out := protoimpl.TypeBuilder{
File: protoimpl.DescBuilder{
GoPackagePath: reflect.TypeOf(x{}).PkgPath(),
RawDescriptor: file_internal_testprotos_race_extender_test_proto_rawDesc,
NumEnums: 0,
NumMessages: 1,
NumExtensions: 1,
NumServices: 0,
},
GoTypes: file_internal_testprotos_race_extender_test_proto_goTypes,
DependencyIndexes: file_internal_testprotos_race_extender_test_proto_depIdxs,
MessageInfos: file_internal_testprotos_race_extender_test_proto_msgTypes,
ExtensionInfos: file_internal_testprotos_race_extender_test_proto_extTypes,
}.Build()
File_internal_testprotos_race_extender_test_proto = out.File
file_internal_testprotos_race_extender_test_proto_rawDesc = nil
file_internal_testprotos_race_extender_test_proto_goTypes = nil
file_internal_testprotos_race_extender_test_proto_depIdxs = nil
}

View File

@ -0,0 +1,19 @@
// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
edition = "2023";
package goproto.proto.test;
import "internal/testprotos/race/message/test.proto";
option go_package = "google.golang.org/protobuf/internal/testprotos/race/extender";
message OtherMessage {
int32 i32 = 1;
}
extend MyMessage {
string s = 2;
}

View File

@ -0,0 +1,145 @@
// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// Code generated by protoc-gen-go. DO NOT EDIT.
// source: internal/testprotos/race/message/test.proto
package message
import (
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
reflect "reflect"
sync "sync"
)
type MyMessage struct {
state protoimpl.MessageState
sizeCache protoimpl.SizeCache
unknownFields protoimpl.UnknownFields
extensionFields protoimpl.ExtensionFields
I32 *int32 `protobuf:"varint,1,opt,name=i32" json:"i32,omitempty"`
}
func (x *MyMessage) Reset() {
*x = MyMessage{}
if protoimpl.UnsafeEnabled {
mi := &file_internal_testprotos_race_message_test_proto_msgTypes[0]
ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x))
ms.StoreMessageInfo(mi)
}
}
func (x *MyMessage) String() string {
return protoimpl.X.MessageStringOf(x)
}
func (*MyMessage) ProtoMessage() {}
func (x *MyMessage) ProtoReflect() protoreflect.Message {
mi := &file_internal_testprotos_race_message_test_proto_msgTypes[0]
if protoimpl.UnsafeEnabled && x != nil {
ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x))
if ms.LoadMessageInfo() == nil {
ms.StoreMessageInfo(mi)
}
return ms
}
return mi.MessageOf(x)
}
// Deprecated: Use MyMessage.ProtoReflect.Descriptor instead.
func (*MyMessage) Descriptor() ([]byte, []int) {
return file_internal_testprotos_race_message_test_proto_rawDescGZIP(), []int{0}
}
func (x *MyMessage) GetI32() int32 {
if x != nil && x.I32 != nil {
return *x.I32
}
return 0
}
var File_internal_testprotos_race_message_test_proto protoreflect.FileDescriptor
var file_internal_testprotos_race_message_test_proto_rawDesc = []byte{
0x0a, 0x2b, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x74, 0x65, 0x73, 0x74, 0x70,
0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2f, 0x72, 0x61, 0x63, 0x65, 0x2f, 0x6d, 0x65, 0x73, 0x73, 0x61,
0x67, 0x65, 0x2f, 0x74, 0x65, 0x73, 0x74, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x12, 0x67,
0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2e, 0x74, 0x65, 0x73,
0x74, 0x22, 0x23, 0x0a, 0x09, 0x4d, 0x79, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x10,
0x0a, 0x03, 0x69, 0x33, 0x32, 0x18, 0x01, 0x20, 0x01, 0x28, 0x05, 0x52, 0x03, 0x69, 0x33, 0x32,
0x2a, 0x04, 0x08, 0x02, 0x10, 0x03, 0x42, 0x3d, 0x5a, 0x3b, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65,
0x2e, 0x67, 0x6f, 0x6c, 0x61, 0x6e, 0x67, 0x2e, 0x6f, 0x72, 0x67, 0x2f, 0x70, 0x72, 0x6f, 0x74,
0x6f, 0x62, 0x75, 0x66, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x74, 0x65,
0x73, 0x74, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2f, 0x72, 0x61, 0x63, 0x65, 0x2f, 0x6d, 0x65,
0x73, 0x73, 0x61, 0x67, 0x65, 0x62, 0x08, 0x65, 0x64, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x70,
0xe8, 0x07,
}
var (
file_internal_testprotos_race_message_test_proto_rawDescOnce sync.Once
file_internal_testprotos_race_message_test_proto_rawDescData = file_internal_testprotos_race_message_test_proto_rawDesc
)
func file_internal_testprotos_race_message_test_proto_rawDescGZIP() []byte {
file_internal_testprotos_race_message_test_proto_rawDescOnce.Do(func() {
file_internal_testprotos_race_message_test_proto_rawDescData = protoimpl.X.CompressGZIP(file_internal_testprotos_race_message_test_proto_rawDescData)
})
return file_internal_testprotos_race_message_test_proto_rawDescData
}
var file_internal_testprotos_race_message_test_proto_msgTypes = make([]protoimpl.MessageInfo, 1)
var file_internal_testprotos_race_message_test_proto_goTypes = []interface{}{
(*MyMessage)(nil), // 0: goproto.proto.test.MyMessage
}
var file_internal_testprotos_race_message_test_proto_depIdxs = []int32{
0, // [0:0] is the sub-list for method output_type
0, // [0:0] is the sub-list for method input_type
0, // [0:0] is the sub-list for extension type_name
0, // [0:0] is the sub-list for extension extendee
0, // [0:0] is the sub-list for field type_name
}
func init() { file_internal_testprotos_race_message_test_proto_init() }
func file_internal_testprotos_race_message_test_proto_init() {
if File_internal_testprotos_race_message_test_proto != nil {
return
}
if !protoimpl.UnsafeEnabled {
file_internal_testprotos_race_message_test_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} {
switch v := v.(*MyMessage); i {
case 0:
return &v.state
case 1:
return &v.sizeCache
case 2:
return &v.unknownFields
case 3:
return &v.extensionFields
default:
return nil
}
}
}
type x struct{}
out := protoimpl.TypeBuilder{
File: protoimpl.DescBuilder{
GoPackagePath: reflect.TypeOf(x{}).PkgPath(),
RawDescriptor: file_internal_testprotos_race_message_test_proto_rawDesc,
NumEnums: 0,
NumMessages: 1,
NumExtensions: 0,
NumServices: 0,
},
GoTypes: file_internal_testprotos_race_message_test_proto_goTypes,
DependencyIndexes: file_internal_testprotos_race_message_test_proto_depIdxs,
MessageInfos: file_internal_testprotos_race_message_test_proto_msgTypes,
}.Build()
File_internal_testprotos_race_message_test_proto = out.File
file_internal_testprotos_race_message_test_proto_rawDesc = nil
file_internal_testprotos_race_message_test_proto_goTypes = nil
file_internal_testprotos_race_message_test_proto_depIdxs = nil
}

View File

@ -0,0 +1,15 @@
// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
edition = "2023";
package goproto.proto.test;
option go_package = "google.golang.org/protobuf/internal/testprotos/race/message";
message MyMessage {
int32 i32 = 1;
extensions 2;
}

View File

@ -0,0 +1,47 @@
// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package race_test
import (
"sync"
"testing"
"google.golang.org/protobuf/proto"
epb "google.golang.org/protobuf/internal/testprotos/race/extender"
mpb "google.golang.org/protobuf/internal/testprotos/race/message"
)
// There must be no other test in this package as we are testing global
// initialization which only happens once per binary.
// It tests that it is safe to initialize the descriptor of a message and
// the descriptor of an extendee of that message in parallel (i.e. no data race).
func TestConcurrentInitialization(t *testing.T) {
var wg sync.WaitGroup
wg.Add(2)
go func() {
defer wg.Done()
m := &mpb.MyMessage{
I32: proto.Int32(int32(42)),
}
// This initializes the descriptor.
_, err := proto.Marshal(m)
if err != nil {
t.Errorf("proto.Marshal(): %v", err)
}
}()
go func() {
defer wg.Done()
m := &epb.OtherMessage{
I32: proto.Int32(int32(42)),
}
// This initializes the descriptor including the extension.
_, err := proto.Marshal(m)
if err != nil {
t.Errorf("proto.Marshal(): %v", err)
}
}()
wg.Wait()
}