Skip to content

Commit 553b33a

Browse files
committed
Address review comments.
- Use GrpcUtil.closeQuietly to close the buffered message in halfClosed() to prevent unnecessary exception propagation if close fails after successful message delivery. - Use stream.cancel instead of call.close when detecting too many requests for unary calls. This ensures the transport is notified to abort the stream (sending RST_STREAM) and immediately releases resources, preventing leaks from clients that withhold END_STREAM.
1 parent 09d8c90 commit 553b33a

2 files changed

Lines changed: 7 additions & 15 deletions

File tree

core/src/main/java/io/grpc/internal/ServerCallImpl.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -352,9 +352,7 @@ private void messagesAvailableInternal(final MessageProducer producer) {
352352
if (call.method.getType().clientSendsOneMessage()) {
353353
if (delayedMessage != null) {
354354
GrpcUtil.closeQuietly(message);
355-
call.close(
356-
Status.INTERNAL.withDescription("Too many requests"),
357-
new Metadata());
355+
call.stream.cancel(Status.INTERNAL.withDescription("Too many requests"));
358356
GrpcUtil.closeQuietly(delayedMessage);
359357
delayedMessage = null;
360358
return;
@@ -401,11 +399,7 @@ public void halfClosed() {
401399
Throwables.throwIfUnchecked(t);
402400
throw new RuntimeException(t);
403401
}
404-
try {
405-
message.close();
406-
} catch (IOException e) {
407-
throw new RuntimeException(e);
408-
}
402+
GrpcUtil.closeQuietly(message);
409403
}
410404

411405
listener.onHalfClose();

core/src/test/java/io/grpc/internal/ServerCallImplTest.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ public void streamListener_messageRead_unary_tooManyRequests() {
571571
// Sending second message should fail
572572
streamListener.messagesAvailable(new SingleMessageProducer(UNARY_METHOD.streamRequest(5678L)));
573573

574-
verify(stream).close(any(Status.class), any(Metadata.class));
574+
verify(stream).cancel(any(Status.class));
575575
verify(callListener, never()).onMessage(any(Long.class));
576576
}
577577

@@ -628,14 +628,12 @@ public void streamListener_halfClosed_closeException() {
628628
// Message should not be delivered yet
629629
verify(callListener, never()).onMessage(any(Long.class));
630630

631-
// halfClosed should throw RuntimeException wrapping IOException
632-
RuntimeException e = assertThrows(RuntimeException.class,
633-
() -> streamListener.halfClosed());
634-
assertThat(e).hasCauseThat().isInstanceOf(IOException.class);
635-
assertThat(e.getCause()).hasMessageThat().isEqualTo("close failed");
631+
// halfClosed should not throw because we use closeQuietly
632+
streamListener.halfClosed();
636633

637-
// The message was delivered before close failed
634+
// The message was delivered and halfClosed completed
638635
verify(callListener).onMessage(1234L);
636+
verify(callListener).onHalfClose();
639637
assertTrue(detachableStream.detachedStream.closed);
640638
}
641639

0 commit comments

Comments
 (0)