desc: fix caching when wrapping protoreflect.Descriptor instances (#606)

Author: jhump

desc/cache.go

@@ -46,12 +46,3 @@ func (c mapCache) get(d protoreflect.Descriptor) Descriptor {
 func (c mapCache) put(key protoreflect.Descriptor, val Descriptor) {
 	c[key] = val
 }
-
-type noopCache struct{}
-
-func (noopCache) get(protoreflect.Descriptor) Descriptor {
-	return nil
-}
-
-func (noopCache) put(protoreflect.Descriptor, Descriptor) {
-}

desc/convert.go

@@ -95,9 +95,7 @@ func convertFile(d protoreflect.FileDescriptor, fd *descriptorpb.FileDescriptorP
 	ret.deps = make([]*FileDescriptor, len(fd.GetDependency()))
 	for i := 0; i < d.Imports().Len(); i++ {
 		f := d.Imports().Get(i).FileDescriptor
-		if c := cache.get(f); c != nil {
-			ret.deps[i] = c.(*FileDescriptor)
-		} else if c, err := wrapFile(f, cache); err != nil {
+		if c, err := wrapFile(f, cache); err != nil {
 			return nil, err
 		} else {
 			ret.deps[i] = c

desc/load.go

@@ -53,13 +53,6 @@ func LoadFileDescriptor(file string) (*FileDescriptor, error) {
 
 	var fd *FileDescriptor
 	loadedDescriptors.withLock(func(cache descriptorCache) {
-		// double-check cache, in case it was concurrently added while
-		// we were waiting for the lock
-		f := cache.get(d)
-		if f != nil {
-			fd = f.(*FileDescriptor)
-			return
-		}
 		fd, err = wrapFile(d, cache)
 	})
 	return fd, err

desc/wrap.go

@@ -21,7 +21,7 @@ type DescriptorWrapper interface {
 // WrapDescriptor wraps the given descriptor, returning a desc.Descriptor
 // value that represents the same element.
 func WrapDescriptor(d protoreflect.Descriptor) (Descriptor, error) {
-	return wrapDescriptor(d, noopCache{})
+	return wrapDescriptor(d, mapCache{})
 }
 
 func wrapDescriptor(d protoreflect.Descriptor, cache descriptorCache) (Descriptor, error) {
@@ -65,18 +65,21 @@ func WrapFiles(d []protoreflect.FileDescriptor) ([]*FileDescriptor, error) {
 // WrapFile wraps the given file descriptor, returning a *desc.FileDescriptor
 // value that represents the same file.
 func WrapFile(d protoreflect.FileDescriptor) (*FileDescriptor, error) {
-	return wrapFile(d, noopCache{})
+	return wrapFile(d, mapCache{})
 }
 
 func wrapFile(d protoreflect.FileDescriptor, cache descriptorCache) (*FileDescriptor, error) {
+	if res := cache.get(d); res != nil {
+		return res.(*FileDescriptor), nil
+	}
 	fdp := protoutil.ProtoFromFileDescriptor(d)
 	return convertFile(d, fdp, cache)
 }
 
 // WrapMessage wraps the given message descriptor, returning a *desc.MessageDescriptor
 // value that represents the same message.
 func WrapMessage(d protoreflect.MessageDescriptor) (*MessageDescriptor, error) {
-	return wrapMessage(d, noopCache{})
+	return wrapMessage(d, mapCache{})
 }
 
 func wrapMessage(d protoreflect.MessageDescriptor, cache descriptorCache) (*MessageDescriptor, error) {
@@ -97,7 +100,7 @@ func wrapMessage(d protoreflect.MessageDescriptor, cache descriptorCache) (*Mess
 // WrapField wraps the given field descriptor, returning a *desc.FieldDescriptor
 // value that represents the same field.
 func WrapField(d protoreflect.FieldDescriptor) (*FieldDescriptor, error) {
-	return wrapField(d, noopCache{})
+	return wrapField(d, mapCache{})
 }
 
 func wrapField(d protoreflect.FieldDescriptor, cache descriptorCache) (*FieldDescriptor, error) {
@@ -121,7 +124,7 @@ func wrapField(d protoreflect.FieldDescriptor, cache descriptorCache) (*FieldDes
 // WrapOneOf wraps the given oneof descriptor, returning a *desc.OneOfDescriptor
 // value that represents the same oneof.
 func WrapOneOf(d protoreflect.OneofDescriptor) (*OneOfDescriptor, error) {
-	return wrapOneOf(d, noopCache{})
+	return wrapOneOf(d, mapCache{})
 }
 
 func wrapOneOf(d protoreflect.OneofDescriptor, cache descriptorCache) (*OneOfDescriptor, error) {
@@ -138,7 +141,7 @@ func wrapOneOf(d protoreflect.OneofDescriptor, cache descriptorCache) (*OneOfDes
 // WrapEnum wraps the given enum descriptor, returning a *desc.EnumDescriptor
 // value that represents the same enum.
 func WrapEnum(d protoreflect.EnumDescriptor) (*EnumDescriptor, error) {
-	return wrapEnum(d, noopCache{})
+	return wrapEnum(d, mapCache{})
 }
 
 func wrapEnum(d protoreflect.EnumDescriptor, cache descriptorCache) (*EnumDescriptor, error) {
@@ -159,7 +162,7 @@ func wrapEnum(d protoreflect.EnumDescriptor, cache descriptorCache) (*EnumDescri
 // WrapEnumValue wraps the given enum value descriptor, returning a *desc.EnumValueDescriptor
 // value that represents the same enum value.
 func WrapEnumValue(d protoreflect.EnumValueDescriptor) (*EnumValueDescriptor, error) {
-	return wrapEnumValue(d, noopCache{})
+	return wrapEnumValue(d, mapCache{})
 }
 
 func wrapEnumValue(d protoreflect.EnumValueDescriptor, cache descriptorCache) (*EnumValueDescriptor, error) {
@@ -176,7 +179,7 @@ func wrapEnumValue(d protoreflect.EnumValueDescriptor, cache descriptorCache) (*
 // WrapService wraps the given service descriptor, returning a *desc.ServiceDescriptor
 // value that represents the same service.
 func WrapService(d protoreflect.ServiceDescriptor) (*ServiceDescriptor, error) {
-	return wrapService(d, noopCache{})
+	return wrapService(d, mapCache{})
 }
 
 func wrapService(d protoreflect.ServiceDescriptor, cache descriptorCache) (*ServiceDescriptor, error) {
@@ -193,7 +196,7 @@ func wrapService(d protoreflect.ServiceDescriptor, cache descriptorCache) (*Serv
 // WrapMethod wraps the given method descriptor, returning a *desc.MethodDescriptor
 // value that represents the same method.
 func WrapMethod(d protoreflect.MethodDescriptor) (*MethodDescriptor, error) {
-	return wrapMethod(d, noopCache{})
+	return wrapMethod(d, mapCache{})
 }
 
 func wrapMethod(d protoreflect.MethodDescriptor, cache descriptorCache) (*MethodDescriptor, error) {