Skip to content

Commit

Permalink
internal/filedesc: defer evaluation of weak reference until actual use
Browse files Browse the repository at this point in the history
Suppose some init logic performs protobuf reflection.
If so, it will cause the lazy descriptor init logic to be executed.
This is problematic for weak fields since we can not be certain that
all weak references have been registered at this point in time.
This is a fundamental issue with with weak dependencies,
since it means that we cannot enforce any ordering constraints
on the weak dependency unless we directly import the weakly referenced package
(which would defeat the point of weak imports).

Alleviate the problem by pushing evaluation of weak reference to
actual usage. This does not completely fix the problem,
but signifcantly reduces the probability of it being problematic.
In general, people should avoid interacting with weak fields at init time.

Change-Id: Ie5957ddedd61333e72ee9a1bba0c53dace65547c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/188982
Reviewed-by: Damien Neil <dneil@google.com>
  • Loading branch information
dsnet committed Aug 5, 2019
1 parent 2aea614 commit f98dd98
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 9 deletions.
18 changes: 14 additions & 4 deletions internal/filedesc/desc.go
Expand Up @@ -16,6 +16,7 @@ import (
"google.golang.org/protobuf/internal/pragma"
"google.golang.org/protobuf/internal/strs"
pref "google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
)

// The types in this file may have a suffix:
Expand Down Expand Up @@ -309,10 +310,19 @@ func (fd *Field) ContainingOneof() pref.OneofDescriptor { return fd.L1.Cont
func (fd *Field) ContainingMessage() pref.MessageDescriptor {
return fd.L0.Parent.(pref.MessageDescriptor)
}
func (fd *Field) Enum() pref.EnumDescriptor { return fd.L1.Enum }
func (fd *Field) Message() pref.MessageDescriptor { return fd.L1.Message }
func (fd *Field) Format(s fmt.State, r rune) { descfmt.FormatDesc(s, r, fd) }
func (fd *Field) ProtoType(pref.FieldDescriptor) {}
func (fd *Field) Enum() pref.EnumDescriptor {
return fd.L1.Enum
}
func (fd *Field) Message() pref.MessageDescriptor {
if fd.L1.IsWeak {
if d, _ := protoregistry.GlobalFiles.FindDescriptorByName(fd.L1.Message.FullName()); d != nil {
return d.(pref.MessageDescriptor)
}
}
return fd.L1.Message
}
func (fd *Field) Format(s fmt.State, r rune) { descfmt.FormatDesc(s, r, fd) }
func (fd *Field) ProtoType(pref.FieldDescriptor) {}

// EnforceUTF8 is a pseudo-internal API to determine whether to enforce UTF-8
// validation for the string field. This exists for Google-internal use only
Expand Down
6 changes: 1 addition & 5 deletions internal/filedesc/desc_lazy.go
Expand Up @@ -32,12 +32,8 @@ func (file *File) resolveMessages() {
for j := range md.L2.Fields.List {
fd := &md.L2.Fields.List[j]

// Weak fields are only resolved by name.
// Weak fields are resolved upon actual use.
if fd.L1.IsWeak {
r := file.builder.FileRegistry
if d, _ := r.FindDescriptorByName(fd.L1.Message.FullName()); d != nil {
fd.L1.Message = d.(pref.MessageDescriptor)
}
continue
}

Expand Down

0 comments on commit f98dd98

Please sign in to comment.