Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
45b5af1
Implement custom events framework in gRPC-Java server
kannanjgithub Aug 3, 2026
e37245c
Fix forwarding listeners to propagate custom events
kannanjgithub Aug 10, 2026
2636ebb
Address review from server interceptor executor design comments.
kannanjgithub Aug 10, 2026
5caf237
Add @ExperimentalApi annotation to triggerEvent and onEvent (issue #1…
kannanjgithub Aug 10, 2026
2158b97
Add unit test for server stream custom events in AbstractTransportTest
kannanjgithub Aug 10, 2026
605cc03
Fix compilation errors in binder, netty, and okhttp (issue #12979)
kannanjgithub Aug 10, 2026
dd549f3
Implement `triggerEvent` for Binder transport.
kannanjgithub Aug 10, 2026
d5f0182
Add unit test coverage for ServerCall triggerEvent and custom events …
kannanjgithub Aug 10, 2026
542f840
Merge remote-tracking branch 'origin/master' into server-framework-cu…
kannanjgithub Aug 11, 2026
958fddc
Fix race condition in AsyncSecurityPoliciesTest.
kannanjgithub Aug 11, 2026
feeab1e
Add unit tests for JTATSSL triggerEvent and cover closed stream event…
kannanjgithub Aug 13, 2026
b9e1e2b
Fix race condition in serverStream_triggerEvent_afterClose test.
kannanjgithub Aug 13, 2026
e832422
Make Binder transport triggerEvent thread-safe and guarantee ordering.
kannanjgithub Aug 31, 2026
48aee63
Propagate custom events in PendingAuthListener, TransmitStatusRuntime…
kannanjgithub Sep 2, 2026
1b8a230
core: add PerfMark tracing and tags to ServerCallImpl and ServerStrea…
kannanjgithub Sep 2, 2026
bdfdafe
core: fast-path return in ServerCallImpl.triggerEvent when closeCalle…
kannanjgithub Sep 3, 2026
b63b4a1
util: add unit tests for TransmitStatusRuntimeExceptionInterceptor cu…
kannanjgithub Sep 3, 2026
91ec839
testing: Fix race condition in MockServerTransportListener and improv…
kannanjgithub Sep 3, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions api/src/main/java/io/grpc/Contexts.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,16 @@ public void onReady() {
context.detach(previous);
}
}

@Override
public void onEvent(Object event) {
Context previous = context.attach();
try {
super.onEvent(event);
} finally {
context.detach(previous);
}
}
}

/**
Expand Down
5 changes: 5 additions & 0 deletions api/src/main/java/io/grpc/PartialForwardingServerCall.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ public SecurityLevel getSecurityLevel() {
return delegate().getSecurityLevel();
}

@Override
public void triggerEvent(Object event) {
delegate().triggerEvent(event);
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this).add("delegate", delegate()).toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ public void onReady() {
delegate().onReady();
}

@Override
public void onEvent(Object event) {
delegate().onEvent(event);
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this).add("delegate", delegate()).toString();
Expand Down
28 changes: 28 additions & 0 deletions api/src/main/java/io/grpc/ServerCall.java
Comment thread
sauravzg marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,20 @@ public void onComplete() {}
* <em>another</em> {@code onReady()} callback.
*/
public void onReady() {}

/**
* A custom event has been triggered by the call.
*
* <p>This callback is guaranteed to run on the call's executor, serialized with other
* callbacks (like {@link #onMessage}, {@link #onHalfClose}). This means the implementation
* does not need internal synchronization to access call-specific state.
*
* @param event the triggered event.
Comment thread
sauravzg marked this conversation as resolved.
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/12979")
public void onEvent(Object event) {
Comment thread
sauravzg marked this conversation as resolved.
// Default no-op
}
}

/**
Expand Down Expand Up @@ -262,6 +276,20 @@ public String getAuthority() {
return null;
}

/**
* Triggers a custom event to be processed by the listener.
* The event will be delivered to {@link Listener#onEvent(Object)} on the call's executor.
*
* <p>This method is thread-safe and can be called from any thread. No events will be delivered
* after the RPC is cancelled or completed.
*
* @param event the event to trigger.
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/12979")
public void triggerEvent(Object event) {
// Default no-op
}

/**
* The {@link MethodDescriptor} for the call.
*/
Expand Down
17 changes: 16 additions & 1 deletion api/src/test/java/io/grpc/ContextsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ public void interceptCall_basic() {
assertSame(uniqueContext, Context.current());
methodCalls.add(5);
}

@Override public void onEvent(Object event) {
assertSame(uniqueContext, Context.current());
methodCalls.add(6);
}
};
ServerCall.Listener<Object> wrapped = interceptCall(uniqueContext, call, headers,
new ServerCallHandler<Object, Object>() {
Expand All @@ -101,7 +106,8 @@ public ServerCall.Listener<Object> startCall(
wrapped.onCancel();
wrapped.onComplete();
wrapped.onReady();
assertEquals(Arrays.asList(1, 2, 3, 4, 5), methodCalls);
wrapped.onEvent(new Object());
assertEquals(Arrays.asList(1, 2, 3, 4, 5, 6), methodCalls);
assertSame(origContext, Context.current());
}

Expand Down Expand Up @@ -145,6 +151,10 @@ public void interceptCall_restoresIfListenerThrows() {
@Override public void onReady() {
throw new RuntimeException();
}

@Override public void onEvent(Object event) {
throw new RuntimeException();
}
};
ServerCall.Listener<Object> wrapped = interceptCall(uniqueContext, call, headers,
new ServerCallHandler<Object, Object>() {
Expand Down Expand Up @@ -180,6 +190,11 @@ public ServerCall.Listener<Object> startCall(
fail("Exception expected");
} catch (RuntimeException expected) {
}
try {
wrapped.onEvent(new Object());
fail("Exception expected");
} catch (RuntimeException expected) {
}
assertSame(origContext, Context.current());
}

Expand Down
11 changes: 11 additions & 0 deletions binder/src/main/java/io/grpc/binder/internal/Inbound.java
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,17 @@ protected void deliverCloseAbnormal(Status status) {
listener.closed(status);
}

void triggerEvent(Object event) {
synchronized (this) {
if (isClosed()) {
return;
}
if (listener != null) {
listener.triggerEvent(event);
}
}
}

@GuardedBy("this")
void onCloseSent(Status status) {
if (!isClosed()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ public void setDecompressor(Decompressor decompressor) {
// Ignore.
}

@Override
public void triggerEvent(Object event) {
inbound.triggerEvent(event);
}

@Override
public void optimizeForDirectExecutor() {
// Ignore.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ public void onReady() {
maybeRunPendingSteps();
}

@Override
public void onEvent(Object event) {
pendingSteps.offer(delegate -> delegate.onEvent(event));
maybeRunPendingSteps();
}

/**
* Similar to Java8's {@link java.util.function.Consumer}, but redeclared in order to support
* Android SDK 21.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ public void setDecompressor(Decompressor decompressor) {
// Ignore.
}

@Override
public void triggerEvent(Object event) {
inbound.triggerEvent(event);
}

@Override
public void optimizeForDirectExecutor() {
// Ignore.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ public ListenableFuture<Status> checkAuthorizationAsync(int uid) {
ListenableFuture<Status> authFuture = asyncPolicy.checkAuthorizationAsync(SOME_UID);
assertThat(awaitResult(settableUid)).isEqualTo(SOME_UID);
authFuture.cancel(false);
executor.submit(() -> {}).get(10, TimeUnit.SECONDS);
Comment thread
sauravzg marked this conversation as resolved.

assertThat(delegateAuthFuture.isCancelled()).isTrue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public void setUp() {
public void onCallbacks_noOpBeforeStartCall() {
listener.onReady();
listener.onMessage("foo");
listener.onEvent("bar");
listener.onHalfClose();
listener.onComplete();

Expand All @@ -54,16 +55,19 @@ public void onCallbacks_noOpBeforeStartCall() {
@Test
public void onCallbacks_runsPendingCallbacksAfterStartCall() {
String message = "foo";
String event = "bar";

// Act 1
listener.onReady();
listener.onMessage(message);
listener.onEvent(event);
listener.startCall(call, headers, next);

// Assert 1
InOrder order = Mockito.inOrder(delegate);
order.verify(delegate).onReady();
order.verify(delegate).onMessage(message);
order.verify(delegate).onEvent(event);

// Act 2
listener.onHalfClose();
Expand Down
17 changes: 17 additions & 0 deletions core/src/main/java/io/grpc/internal/AbstractServerStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,16 @@ public final void setListener(ServerStreamListener serverStreamListener) {
transportState().setListener(serverStreamListener);
}

@Override
public final void triggerEvent(final Object event) {
transportState().runOnTransportThread(new Runnable() {
@Override
public void run() {
transportState().triggerEvent(event);
}
});
}

@Override
public StatsTraceContext statsTraceContext() {
return statsTraceCtx;
Expand Down Expand Up @@ -259,6 +269,13 @@ public void deframerClosed(boolean hasPartialMessage) {



public final void triggerEvent(Object event) {
if (listenerClosed) {
return;
}
listener().triggerEvent(event);
}

@Override
protected ServerStreamListener listener() {
return listener;
Expand Down
22 changes: 22 additions & 0 deletions core/src/main/java/io/grpc/internal/ServerCallImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,17 @@ public MethodDescriptor<ReqT, RespT> getMethodDescriptor() {
return method;
}

@Override
public void triggerEvent(Object event) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need a cancellation and close check here? other methods seem to have it.

closeCalled is interesting because it'd require us to make it volatile which may break other assumptions about thread safety.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A check in ServerCallImpl could never prevent concurrent races anyway. Because ServerCall.triggerEvent is intended to be thread-safe, external threads may invoke call.triggerEvent(...) concurrently with the application thread invoking call.close(...).

  • Even if closeCalled were volatile, thread A could read closeCalled == false a nanosecond before thread B sets closeCalled = true.
  • An event could therefore always enter stream.triggerEvent(...) while close is in progress.
  • Thus, the authoritative synchronization point must reside in the transport layer where stream lifecycle state and inbound events are serialize.

The listenerClosed check in AbstractServerStream.triggerEvent completely compensates for not checking closeCalled in ServerCallImpl.triggerEvent, by dropping the event if the listener is closed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am less worried about races since we can fix and address them and more worried about the expected behavior.

Right now it seems like we don't check cancellation or closure status here when triggering operations which may be okay if our interface contract is "you should not call after cancellation" . If our contract allows or specifies the behavior after cancellation , I'd assume we check and enforce it here in the implementation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ServerCall.close's javadoc says

This method implies the caller completed processing the RPC

so that implies that a triggerEvent called after onClose will be ignored.
My earlier reply was concerning races but I agree that we can simply return if callClosed is true when triggerEvent is called, since it provides a fast-path optimization when the same thread invokes call.triggerEvent after invoking call.close().

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unresolving. While looking at it, I realized that closeCalled is non volatile and being used in a thread safe method. I'd guess this is bad behavior?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thread is using a thread-local variable, it is not even meant for cross thread visibility. So it doesn't fit into the definition you are quoting at all.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might not be on the same page here? closeCalled is not a thread local. I think we can take this offline and discuss tommorow.

@kannanjgithub kannanjgithub Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not thread-local. What I really meant was that the only scenario where it helps is intra-thread offering a fast path. Where it is visible to other threads (like a volatile), it still does nothing about preventing races.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do this.

Let's try to change the test so that call.close and call.triggerEvent run concurrently on different threads instead of sequentially like they are right now and run a tsan on it. IIUC, that's a very valid behavior that an application can exhibit right? We'll know based on if TSAN fails or not.

  public void triggerEvent_afterClose_noop() {
    call.close(Status.OK, new Metadata());
    Object event = new Object();
    call.triggerEvent(event);
    verify(stream, never()).triggerEvent(event);
  }

@kannanjgithub kannanjgithub Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TSAN will flag it as a race, I'm not contesting that. But there is a reason TSAN suppressions exist (ex. CL/967859705) for developers to indicate intentionally allowed races that have no adverse impact.

Comment thread
sauravzg marked this conversation as resolved.
if (closeCalled) {
return;
}
try (TaskCloseable ignore = PerfMark.traceTask("ServerCall.triggerEvent")) {
PerfMark.attachTag(tag);
stream.triggerEvent(event);
}
}

@Override
public SecurityLevel getSecurityLevel() {
final Attributes attributes = getAttributes();
Expand Down Expand Up @@ -395,5 +406,16 @@ public void onReady() {
listener.onReady();
}
}

@Override
public void triggerEvent(Object event) {
try (TaskCloseable ignore = PerfMark.traceTask("ServerStreamListener.triggerEvent")) {
PerfMark.attachTag(call.tag);
if (call.cancelled) {
return;
}
listener.onEvent(event);
}
}
}
}
31 changes: 31 additions & 0 deletions core/src/main/java/io/grpc/internal/ServerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,9 @@ public void closed(Status status) {}

@Override
public void onReady() {}

@Override
public void triggerEvent(Object event) {}
}

/**
Expand Down Expand Up @@ -960,6 +963,34 @@ public void runInContext() {
callExecutor.execute(new OnReady());
}
}

@Override
public void triggerEvent(final Object event) {
try (TaskCloseable ignore = PerfMark.traceTask("ServerStreamListener.triggerEvent")) {
PerfMark.attachTag(tag);
final Link link = PerfMark.linkOut();

final class TriggerEvent extends ContextRunnable {
TriggerEvent() {
super(context);
}

@Override
public void runInContext() {
try (TaskCloseable ignore = PerfMark.traceTask("ServerCallListener(app).onEvent")) {
PerfMark.attachTag(tag);
PerfMark.linkIn(link);
getListener().triggerEvent(event);
} catch (Throwable t) {
internalClose(t);
throw t;
}
}
}

callExecutor.execute(new TriggerEvent());
}
}
}

@VisibleForTesting
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/java/io/grpc/internal/ServerStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ public interface ServerStream extends Stream {
*/
void setListener(ServerStreamListener serverStreamListener);

/**
* Triggers a custom event. Implementations must ensure this is propagated to the
* listener on the transport thread.
*/
void triggerEvent(Object event);

/**
* The context for recording stats and traces for this stream.
*/
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/io/grpc/internal/ServerStreamListener.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,9 @@ public interface ServerStreamListener extends StreamListener {
* @param status details about the remote closure
*/
void closed(Status status);

/**
* Propagates a custom event to the listener. Must be called on the transport thread.
*/
void triggerEvent(Object event);
}
28 changes: 28 additions & 0 deletions core/src/test/java/io/grpc/internal/AbstractServerStreamTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,31 @@ public void close_sendTrailersClearsReservedFields() {
assertEquals("bad", metadataCaptor.getValue().get(InternalStatus.MESSAGE_KEY));
}

@Test
public void triggerEvent_propagatesToListener() {
ServerStreamListener listener = mock(ServerStreamListener.class);
stream.transportState().setListener(listener);

Object event = new Object();
stream.triggerEvent(event);

verify(listener).triggerEvent(event);
}

@Test
public void triggerEvent_ignoredAfterClose() {
ServerStreamListener listener = mock(ServerStreamListener.class);
stream.transportState().setListener(listener);

stream.close(Status.OK, new Metadata());
stream.transportState().complete();

Object event = new Object();
stream.triggerEvent(event);

verify(listener, never()).triggerEvent(any());
}

@Test
public void changeOnReadyThreshold() {
stream.setListener(new ServerStreamListenerBase());
Expand Down Expand Up @@ -391,6 +416,9 @@ public void halfClosed() {}

@Override
public void closed(Status status) {}

@Override
public void triggerEvent(Object event) {}
}

private static class AbstractServerStreamBase extends AbstractServerStream {
Expand Down
Loading
Loading