jjava StreamObserver thread safety

625 views
Skip to first unread message

Steven Parkes

unread,
Apr 6, 2016, 3:57:32 PM4/6/16
to grp...@googlegroups.com
Are StreamObservers thread-safe? In particular, can onNext be called concurrently on a single StreamObserver from multiple threads?

The docs say

Implementations are expected to be thread-compatible. Separate StreamObservers do not need to be sychronized (SIC) together; incoming and outgoing directions are independent.
 
It's unclear (to me, anyway) what thread-compatible means, if it's only meant to apply between the StreamObservers for a single RPC.

I'm getting an NPE io.grpc.internal.ServerCallImpl.sendHeaders, possibly because I have multiple threads writing to the same observer. It's easy enough to synchronize and that appears to solve the problem, I just want to make sure it's the right thing to do ...

Louis Ryan

unread,
Apr 7, 2016, 1:53:55 PM4/7/16
to Steven Parkes, grpc-io
A single stream observer is not thread safe. The documentation is pointing out that the two stream observers which represent the input and output of a call are fully independent and so no cross-synchronization need occur.

In your case you have many writers to a single stream observer and so you should synchronize. 

Suggestions on how to improve the documentation always welcome (as Github issues)

--
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/CAMAOpmUaGXamct8sBJwp8OYj3j_4zksCoK_S4sguXZhre6a-pw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Steven Parkes

unread,
Apr 7, 2016, 1:59:56 PM4/7/16
to Louis Ryan, grpc-io
Pretty much what I deduced. Thanks for the clarification.

I'll submit a small wording change.

Steven Parkes

unread,
Apr 7, 2016, 2:13:21 PM4/7/16
to Louis Ryan, grpc-io
Ah, and I see Eric answered this back in September: https://groups.google.com/forum/#!msg/grpc-io/tK8qyVn9rm4/MFgocH3IEAAJ

Eric Anderson

unread,
Apr 7, 2016, 3:29:44 PM4/7/16
to Steven Parkes, grpc-io
On Wed, Apr 6, 2016 at 12:57 PM, Steven Parkes <smpa...@smparkes.net> wrote:
The docs say

Implementations are expected to be thread-compatible. Separate StreamObservers do not need to be sychronized (SIC) together; incoming and outgoing directions are independent.
 
It's unclear (to me, anyway) what thread-compatible means, if it's only meant to apply between the StreamObservers for a single RPC.

thread-compatible is a fancy but more precise way of saying "not thread safe." It's more precise because it means a different thread can be used each call and other things we typically take for granted. ThreadLocal may be different between calls, and the code can't use static data without making those accesses thread-safe.

I searched around and this page looks like it has a reasonable introduction.

Steven Parkes

unread,
Apr 7, 2016, 3:37:45 PM4/7/16
to Eric Anderson, grpc-io
TIL.

Nice description. I thought I did a search for it before I posted but given that it's the second Google hit, I'm thinking I didn't.

I'd be inclined to actually link that article in the comment ... but don't know what you want in javadoc style. I'll add it to the PR if you're +1.

That said, I'd say it's a bit of a niche term and I'd be +1 to be a bit more explicit given that the term isn't so wide spread (though should be ...)

Eric Anderson

unread,
Apr 7, 2016, 3:48:01 PM4/7/16
to Steven Parkes, grpc-io
Including a link sounds fine. I don't think I'd mind if we even just remove the thread-compatible wording and just say something along the lines of "not thread safe" (but still include some detail about sending/receiving being independent). I'm the one who added that sentence, but I did it in passing and a bit rushed, but I had noticed the docs said nothing about threading requirements at the time and knew we needed something.

We can continue discussion in the PR you created.
Reply all
Reply to author
Forward
0 new messages