Skip to content

Commit c6cd8f5

Browse files
committed
binder: Don't report onReady() for a closed Inbound
Inbound.onTransportReady() unconditionally forwarded onReady() to the listener, so flow control unblocking concurrently with a close could invoke onReady() after closed(), which must be the listener's final callback. Only skip the listener, not the Outbound: a server stream's close may still be sitting in its Outbound waiting for the transmit window to open.
1 parent 98fe45a commit c6cd8f5

2 files changed

Lines changed: 20 additions & 1 deletion

File tree

‎binder/src/main/java/io/grpc/binder/internal/Inbound.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,13 @@ final void onTransportReady() {
313313
Outbound outbound = null;
314314
StreamListener listener = null;
315315
synchronized (this) {
316+
// closed() is always the listener's final callback.
317+
if (!isClosed()) {
318+
listener = this.listener;
319+
}
320+
// But poke the Outbound even if we're closed since a server stream's close could be sitting
321+
// in it, blocked on flow control.
316322
outbound = this.outbound;
317-
listener = this.listener;
318323
}
319324
if (listener != null) {
320325
listener.onReady();

‎binder/src/test/java/io/grpc/binder/internal/ClientInboundTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,20 @@ public void transactionsIgnoredAfterClosed() throws Exception {
410410
assertThat(listener.getReadMessages()).isEmpty();
411411
}
412412

413+
@Test
414+
public void transportReadyAfterClosed_doesNotReportOnReady() throws Exception {
415+
newStreamTxnToClientBuilder(nextTxIndex++).withPrefix(new Metadata()).dispatchTo(inbound);
416+
newOutOfBandCloseTxnBuilder(Status.CANCELLED).dispatchTo(inbound);
417+
drainExecutors();
418+
assertThat(listener.isClosed()).isTrue();
419+
420+
// Simulates BinderTransport.handleAcknowledgedBytes() picking this Inbound out of ongoingCalls
421+
// just before a concurrent close (e.g. cancel()) unregistered it. FakeClientStreamListener
422+
// throws if onReady() is invoked after closed().
423+
inbound.onTransportReady();
424+
drainExecutors();
425+
}
426+
413427
@Test
414428
public void allInOneUnaryTransaction() throws Exception {
415429
newStreamTxnToClientBuilder(0)

0 commit comments

Comments
 (0)