reflect/protoregistry: protect global registries with a lock
The global registry is initialized via generated code.
The Go language guarantees that these are serialized (non concurrently).
The main concern is when a concurrent read operation occurs while
registration is still ongoing. In such a case, we do need a lock to
serialize the read with regard to the writes (i.e. registrations).
Change-Id: Ied35d6f8d2620f448cb281c3ec46d8de893b5671
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/199217
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/reflect/protoregistry/registry.go b/reflect/protoregistry/registry.go
index a4df65b..cba713f 100644
--- a/reflect/protoregistry/registry.go
+++ b/reflect/protoregistry/registry.go
@@ -20,6 +20,7 @@
"log"
"reflect"
"strings"
+ "sync"
"google.golang.org/protobuf/internal/errors"
"google.golang.org/protobuf/reflect/protoreflect"
@@ -37,6 +38,8 @@
return true
}
+var globalMutex sync.RWMutex
+
// GlobalFiles is a global registry of file descriptors.
var GlobalFiles *Files = new(Files)
@@ -87,6 +90,10 @@
//
// It is permitted for multiple files to have the same file path.
func (r *Files) Register(files ...protoreflect.FileDescriptor) error {
+ if r == GlobalFiles {
+ globalMutex.Lock()
+ defer globalMutex.Unlock()
+ }
if r.descsByName == nil {
r.descsByName = map[protoreflect.FullName]interface{}{
"": &packageDescriptor{},
@@ -161,6 +168,10 @@
if r == nil {
return nil, NotFound
}
+ if r == GlobalFiles {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
prefix := name
suffix := nameSuffix("")
for prefix != "" {
@@ -249,6 +260,10 @@
if r == nil {
return nil, NotFound
}
+ if r == GlobalFiles {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
if fd, ok := r.filesByPath[path]; ok {
return fd, nil
}
@@ -260,6 +275,10 @@
if r == nil {
return 0
}
+ if r == GlobalFiles {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
return len(r.filesByPath)
}
@@ -269,6 +288,10 @@
if r == nil {
return
}
+ if r == GlobalFiles {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
for _, file := range r.filesByPath {
if !f(file) {
return
@@ -281,6 +304,10 @@
if r == nil {
return 0
}
+ if r == GlobalFiles {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
p, ok := r.descsByName[name].(*packageDescriptor)
if !ok {
return 0
@@ -294,6 +321,10 @@
if r == nil {
return
}
+ if r == GlobalFiles {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
p, ok := r.descsByName[name].(*packageDescriptor)
if !ok {
return
@@ -441,6 +472,10 @@
// (e.g., two different types have the same full name),
// then the first type takes precedence and an error is returned.
func (r *Types) Register(typs ...Type) error {
+ if r == GlobalTypes {
+ globalMutex.Lock()
+ defer globalMutex.Unlock()
+ }
var firstErr error
typeLoop:
for _, typ := range typs {
@@ -525,6 +560,10 @@
if r == nil {
return nil, NotFound
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
if v := r.typesByName[enum]; v != nil {
if et, _ := v.(protoreflect.EnumType); et != nil {
return et, nil
@@ -551,6 +590,10 @@
if r == nil {
return nil, NotFound
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
message := protoreflect.FullName(url)
if i := strings.LastIndexByte(url, '/'); i >= 0 {
message = message[i+len("/"):]
@@ -575,6 +618,10 @@
if r == nil {
return nil, NotFound
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
if v := r.typesByName[field]; v != nil {
if xt, _ := v.(protoreflect.ExtensionType); xt != nil {
return xt, nil
@@ -592,6 +639,10 @@
if r == nil {
return nil, NotFound
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
if xt, ok := r.extensionsByMessage[message][field]; ok {
return xt, nil
}
@@ -603,6 +654,10 @@
if r == nil {
return 0
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
return r.numEnums
}
@@ -612,6 +667,10 @@
if r == nil {
return
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
for _, typ := range r.typesByName {
if et, ok := typ.(protoreflect.EnumType); ok {
if !f(et) {
@@ -626,6 +685,10 @@
if r == nil {
return 0
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
return r.numMessages
}
@@ -635,6 +698,10 @@
if r == nil {
return
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
for _, typ := range r.typesByName {
if mt, ok := typ.(protoreflect.MessageType); ok {
if !f(mt) {
@@ -649,6 +716,10 @@
if r == nil {
return 0
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
return r.numExtensions
}
@@ -658,6 +729,10 @@
if r == nil {
return
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
for _, typ := range r.typesByName {
if xt, ok := typ.(protoreflect.ExtensionType); ok {
if !f(xt) {
@@ -673,6 +748,10 @@
if r == nil {
return 0
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
return len(r.extensionsByMessage[message])
}
@@ -682,6 +761,10 @@
if r == nil {
return
}
+ if r == GlobalTypes {
+ globalMutex.RLock()
+ defer globalMutex.RUnlock()
+ }
for _, xt := range r.extensionsByMessage[message] {
if !f(xt) {
return
