41

While working and generating protobuf stubs in go I stumbled upon this interesting issue.

Whenever I try and copy a message's struct by value I get this warning:

call of state.world.script.HandleEvent copies lock value: throne/server/messages.PlayerDialogeStatus contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutex copylocks

While I understand why copying a mutex lock by value is wrong, I started wondering why are they even there in the first place.

And thus my question: Why does the go generated protobuf files contain mutex locks placed on the message structs, specifically on the MessageState struct?

Or alternatively: What is the goal of the mutex lock placed in the MessageState struct found on generated protobuf message structs?

Improve this question

2 Answers 2

Reset to default
27

The impl.MessageState is embedded in concrete messages only, not in the generated structs that implement a proto message.

It specifically embeds the three pragmas: NoUnkeyedLiterals, DoNotCompare, and DoNotCopy.

The last one, DoNotCopy is a zero-sized array of sync.Mutex. The sole purpose is to have go vet complain loudly about shallow copies, as described in the comment:

DoNotCopy can be embedded in a struct to help prevent shallow copies. This does not rely on a Go language feature, but rather a special case within the vet checker.

The summary of it all: impl.MessageState is not supposed to be copied and the mutex is there only to catch copying. If you do so, it is because you are using something the wrong way.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

If you do so, it is because you are using something the wrong way. Can you expand on that? What is the downside of copying it? What sorts of problems does it invite?
I also would like to learn what I was doing wrong by passing proto structures by value. This change caused massive inconvenience in upgrading to the newest version of proto in our project. I prefer not to deal with nils if possible and rely on the empty default value.
In addition to Jeff Widman's good question. If one wishes to copy the message state, what is the "right" way to go about it?
15

As far as I can tell, there are three reasons the Go protobuf API includes the DoNotCopy mutex:

  1. The maintainers may wish to change the internal representation in the future, in a way that will not work with shallow copies.
  2. It is theoretically unsafe to mix atomic and non-atomic accesses to the same memory. The protobuf structs contains an internal field that is usually read and written with an atomic access. Calling proto.Marshal(msg), then overwriting the contents with *msg = MyMessage{...} mixes atomic and non-atomic accesses. Even if this works with today's implementation on x86, there is no guarantee this will work on other systems in the future. (See a long Go issue about this).
  3. If you call ProtoReflect() on a message, then later overwrite the message, it will crash, since the ProtoReflect() result relies on the internal reflection pointer (original issue):
d := &durationpb.Duration{Seconds: 1}
protoreflectMessage := d.ProtoReflect()
fmt.Printf("protoreflectMessage.Interface()=%v\n", protoreflectMessage.Interface())
*d = durationpb.Duration{Seconds: 2}
fmt.Printf("protoreflectMessage.Interface()=%v\n", protoreflectMessage.Interface())

This crashes with:

protoreflectMessage.Interface()=seconds:1
panic: invalid nil message info; this suggests memory corruption due to a race or shallow copy on the message struct
Improve this answer

2 Comments

I don't see Marshall() in my messages generated code. How it is supposed to cause atomic access ?
I have edited the answer: it is not msg.Marshall(), it is calling the proto.Marshal function on the message. I hope that clarifies the potential problems.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.