From bc2e1764f6096a9a39fbc37043fb7c266381e6d5 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Tue, 28 May 2019 17:52:39 -0700 Subject: [PATCH] api,stub: Clarify isReady()/onReady() interaction semantics --- api/src/main/java/io/grpc/ClientCall.java | 15 ++++++++++++--- api/src/main/java/io/grpc/ServerCall.java | 14 ++++++++++++-- .../java/io/grpc/stub/CallStreamObserver.java | 10 ++++++++-- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/io/grpc/ClientCall.java b/api/src/main/java/io/grpc/ClientCall.java index 2aa5034b7b..234678618e 100644 --- a/api/src/main/java/io/grpc/ClientCall.java +++ b/api/src/main/java/io/grpc/ClientCall.java @@ -144,11 +144,17 @@ public abstract class ClientCall { public void onClose(Status status, Metadata trailers) {} /** - * This indicates that the ClientCall is now capable of sending additional messages (via + * This indicates that the ClientCall may now be capable of sending additional messages (via * {@link #sendMessage}) without requiring excessive buffering internally. This event is * just a suggestion and the application is free to ignore it, however doing so may * result in excessive buffering within the ClientCall. * + *

Because there is a processing delay to deliver this notification, it is possible for + * concurrent writes to cause {@code isReady() == false} within this callback. Handle "spurious" + * notifications by checking {@code isReady()}'s current value instead of assuming it is now + * {@code true}. If {@code isReady() == false} the normal expectations apply, so there would be + * another {@code onReady()} callback. + * *

If the type of a call is either {@link MethodDescriptor.MethodType#UNARY} or * {@link MethodDescriptor.MethodType#SERVER_STREAMING}, this callback may not be fired. Calls * that send exactly one message should not await this callback. @@ -236,12 +242,15 @@ public abstract class ClientCall { * just a suggestion and the application is free to ignore it, however doing so may * result in excessive buffering within the call. * - *

This abstract class's implementation always returns {@code true}. Implementations generally - * override the method. + *

If {@code false}, {@link Listener#onReady()} will be called after {@code isReady()} + * transitions to {@code true}. * *

If the type of the call is either {@link MethodDescriptor.MethodType#UNARY} or * {@link MethodDescriptor.MethodType#SERVER_STREAMING}, this method may persistently return * false. Calls that send exactly one message should not check this method. + * + *

This abstract class's implementation always returns {@code true}. Implementations generally + * override the method. */ public boolean isReady() { return true; diff --git a/api/src/main/java/io/grpc/ServerCall.java b/api/src/main/java/io/grpc/ServerCall.java index d91f751cb5..15ac0efdc9 100644 --- a/api/src/main/java/io/grpc/ServerCall.java +++ b/api/src/main/java/io/grpc/ServerCall.java @@ -83,10 +83,16 @@ public abstract class ServerCall { public void onComplete() {} /** - * This indicates that the call is now capable of sending additional messages (via + * This indicates that the call may now be capable of sending additional messages (via * {@link #sendMessage}) without requiring excessive buffering internally. This event is * just a suggestion and the application is free to ignore it, however doing so may * result in excessive buffering within the call. + * + *

Because there is a processing delay to deliver this notification, it is possible for + * concurrent writes to cause {@code isReady() == false} within this callback. Handle "spurious" + * notifications by checking {@code isReady()}'s current value instead of assuming it is now + * {@code true}. If {@code isReady() == false} the normal expectations apply, so there would be + * another {@code onReady()} callback. */ public void onReady() {} } @@ -133,7 +139,11 @@ public abstract class ServerCall { * just a suggestion and the application is free to ignore it, however doing so may * result in excessive buffering within the call. * - *

This implementation always returns {@code true}. + *

If {@code false}, {@link Listener#onReady()} will be called after {@code isReady()} + * transitions to {@code true}. + * + *

This abstract class's implementation always returns {@code true}. Implementations generally + * override the method. */ public boolean isReady() { return true; diff --git a/stub/src/main/java/io/grpc/stub/CallStreamObserver.java b/stub/src/main/java/io/grpc/stub/CallStreamObserver.java index 8bcfb2151a..8ed16bb96e 100644 --- a/stub/src/main/java/io/grpc/stub/CallStreamObserver.java +++ b/stub/src/main/java/io/grpc/stub/CallStreamObserver.java @@ -48,6 +48,9 @@ public abstract class CallStreamObserver implements StreamObserver { * without requiring excessive buffering internally. This value is just a suggestion and the * application is free to ignore it, however doing so may result in excessive buffering within the * observer. + * + *

If {@code false}, the runnable passed to {@link #setOnReadyHandler} will be called after + * {@code isReady()} transitions to {@code true}. */ public abstract boolean isReady(); @@ -61,8 +64,11 @@ public abstract class CallStreamObserver implements StreamObserver { * ClientResponseObserver#beforeStart}. On server-side it may only be called during the initial * call to the application, before the service returns its {@code StreamObserver}. * - *

Note that the handler may be called some time after {@link #isReady} has transitioned to - * true as other callbacks may still be executing in the 'inbound' observer. + *

Because there is a processing delay to deliver this notification, it is possible for + * concurrent writes to cause {@code isReady() == false} within this callback. Handle "spurious" + * notifications by checking {@code isReady()}'s current value instead of assuming it is now + * {@code true}. If {@code isReady() == false} the normal expectations apply, so there would be + * another {@code onReadyHandler} callback. * * @param onReadyHandler to call when peer is ready to receive more messages. */