grpc-go: Stream objects are not thread-safe

[tsuna]

Hi,
I couldn’t find any mention in the doc as to whether or not it’s safe
to call Send() from multiple goroutines concurrently on a stream
object. It appears that I mistakenly assumed that it was, but it’s
not.

Send() calls x.ServerStream.SendMsg(m) in the protoc-generated code.
This takes us here:
https://github.com/grpc/grpc-go/blob/89f694edb447e224bd0ffff7a03f9161ce486482/stream.go#L198

where we do:
out, err := encode(cs.codec, m, cs.cp, cs.cbuf)

and encode() is going to write into the bytes.Buffer passed as the
last argument (cs.cbuf) and no lock is acquired in the process.

So two goroutines calling Send() concurrently will step over each
other’s toes and lead to a corrupted protobuf being sent.

Took us a while to figure that one out as the race detector doesn’t
detect this one for some reason.

Before filing an issue on GitHub I just wanted to check whether this
was indeed the intended behavior and we have to add our own locking
around the Stream objects?

--
Benoit "tsuna" Sigoure

[Qi Zhao]

stream does not allow multiple concurrent writers. The general rule is that you should NOT assume it is safe to be called concurrently unless the comment explicitly states that (you can go to check some standard golang libraries (e.g., container/list) to see whether they explicitly state they do not support concurrent op when they are not).

--
Benoit "tsuna" Sigoure

--
You received this message because you are subscribed to the Google Groups "grpc.io" group.
To unsubscribe from this group and stop receiving emails from it, send an email to grpc-io+unsubscribe@googlegroups.com.
To post to this group, send email to grp...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/CAFKYj4dr4CSQ1SFVfs4XvOHJWzxSa5ZBWGXDmq6CsskXNo0Saw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

Thanks,

-Qi

[GoldenBull Chen]

Can multi threads write to a stream which is protected by a lock?

To unsubscribe from this group and stop receiving emails from it, send an email to grpc-io+u...@googlegroups.com.

To post to this group, send email to grp...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/CAFKYj4dr4CSQ1SFVfs4XvOHJWzxSa5ZBWGXDmq6CsskXNo0Saw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

Thanks,

-Qi

--

You received this message because you are subscribed to the Google Groups "grpc.io" group.

To unsubscribe from this group and stop receiving emails from it, send an email to grpc-io+u...@googlegroups.com.

To post to this group, send email to grp...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/CAFnDmdok75FCokH2SCw2xgiok12kADpEWXCiwGWEiJa-73QCbg%40mail.gmail.com.

[tsuna]

On Thu, Feb 25, 2016 at 6:07 PM, Qi Zhao <zh...@google.com> wrote:
> stream does not allow multiple concurrent writers. The general rule is that
> you should NOT assume it is safe to be called concurrently unless the
> comment explicitly states that (you can go to check some standard golang
> libraries (e.g., container/list) to see whether they explicitly state they
> do not support concurrent op when they are not).

Sure, that’s totally reasonable. Thanks for clarifying.

On Thu, Feb 25, 2016 at 7:47 PM, GoldenBull Chen <golde...@gmail.com> wrote:
> Can multi threads write to a stream which is protected by a lock?

From my reading of the code, yes. At least I wasn’t able to reproduce
the problem once I wrapped the stream with a sync.Mutex.

--
Benoit "tsuna" Sigoure