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>
This commit is contained in:
Joe Tsai 2019-07-06 18:08:55 -07:00
parent 93fd968b71
commit 8a4c3d18b1
2 changed files with 13 additions and 10 deletions

View File

@ -6,6 +6,7 @@ package filedesc
import (
"reflect"
"sync"
"google.golang.org/protobuf/internal/descopts"
"google.golang.org/protobuf/internal/encoding/wire"
@ -675,14 +676,18 @@ func (db *DescBuilder) optionsUnmarshaler(p pref.ProtoMessage, b []byte) func()
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
}
}

View File

@ -85,8 +85,6 @@ type Descriptor interface {
// 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.
//