internal/filedesc: avoid deep-copying the options
The protoreflect.Descriptor.Options method is currently documented as
returning a reference to the options, where the user must not mutate
the returned message. This changes internal/filedesc to avoid returning
a copy of the options by caching the first unmarshal.
See golang/protobuf#877
Change-Id: I15701d33fbda7535b21b2add72628b02992c373f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185197
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
diff --git a/internal/filedesc/desc_lazy.go b/internal/filedesc/desc_lazy.go
index 9fc5b1b..4d31109 100644
--- a/internal/filedesc/desc_lazy.go
+++ b/internal/filedesc/desc_lazy.go
@@ -6,6 +6,7 @@
import (
"reflect"
+ "sync"
"google.golang.org/protobuf/internal/descopts"
"google.golang.org/protobuf/internal/encoding/wire"
@@ -675,14 +676,18 @@
if b == nil {
return nil
}
+ var opts pref.ProtoMessage
+ var once sync.Once
return func() pref.ProtoMessage {
- p := reflect.New(reflect.TypeOf(p).Elem()).Interface().(pref.ProtoMessage)
- if err := (proto.UnmarshalOptions{
- AllowPartial: true,
- Resolver: db.TypeResolver,
- }).Unmarshal(b, p); err != nil {
- panic(err)
- }
- return p
+ once.Do(func() {
+ opts = reflect.New(reflect.TypeOf(p).Elem()).Interface().(pref.ProtoMessage)
+ if err := (proto.UnmarshalOptions{
+ AllowPartial: true,
+ Resolver: db.TypeResolver,
+ }).Unmarshal(b, opts); err != nil {
+ panic(err)
+ }
+ })
+ return opts
}
}
diff --git a/reflect/protoreflect/type.go b/reflect/protoreflect/type.go
index a733be1..d252caf 100644
--- a/reflect/protoreflect/type.go
+++ b/reflect/protoreflect/type.go
@@ -85,8 +85,6 @@
// For FileDescriptor, the Path and Package are also valid.
IsPlaceholder() bool
- // TODO: Decide memory ownership of Options.
-
// Options returns the descriptor options. The caller must not modify
// the returned value.
//
