fix(grpc): keep connection alive on client send half-close#670
Merged
fix(grpc): keep connection alive on client send half-close#670
Conversation
the ping patch (`91709fd`, `str-300: do no skip ping logic even if the channel is full`) removed `ping_client_tx`, which was a clone of `client_tx` that lived inside the ping task. it was removed because pings no longer needed to signal disconnect explicitly, the new `.send().await` approach just breaks on error. removing that clone had a side effect. before the patch there were two senders for the `client_rx` channel: `ping_client_tx` in the ping task, and `incoming_client_tx` in the incoming message task. an `mpsc` channel only closes when all senders are dropped. when a client half-closed its send stream (normal in grpc, it just means "i have nothing more to send"), the incoming message task would receive `Ok(None)`, break, and drop `incoming_client_tx`. but `ping_client_tx` was still alive in the ping task, so `client_rx` stayed open and `client_loop` kept running. the client kept receiving data. this is correct for grpc bidirectional streaming, half-closing one direction should not tear down the other. after the patch, `incoming_client_tx` is the only sender. when the incoming task breaks on `Ok(None)`, all senders are gone, `client_rx.recv()` returns `None`, and `client_loop` treats that as a disconnect. the entire connection is torn down because the client closed its send half. this breaks any client that sends its initial filter and then drops the send side of the stream, which is a common pattern. most simple clients send one `SubscribeRequest` and just read. in grpc, a client half-close is not a disconnect, it just means the client is done sending. the server should keep streaming. the fix is in the `Ok(None)` handler of the incoming message task. instead of breaking immediately (which drops the sender and cascades into a full disconnect), the task now awaits `incoming_cancellation_token.cancelled()`. this keeps `incoming_client_tx` alive so `client_loop` continues running until an actual disconnect condition happens like server shutdown or stream error.
lvboudre
approved these changes
Feb 4, 2026
WilfredAlmeida
pushed a commit
that referenced
this pull request
Feb 5, 2026
* fix(grpc): keep connection alive on client send half-close the ping patch (`91709fd`, `str-300: do no skip ping logic even if the channel is full`) removed `ping_client_tx`, which was a clone of `client_tx` that lived inside the ping task. it was removed because pings no longer needed to signal disconnect explicitly, the new `.send().await` approach just breaks on error. removing that clone had a side effect. before the patch there were two senders for the `client_rx` channel: `ping_client_tx` in the ping task, and `incoming_client_tx` in the incoming message task. an `mpsc` channel only closes when all senders are dropped. when a client half-closed its send stream (normal in grpc, it just means "i have nothing more to send"), the incoming message task would receive `Ok(None)`, break, and drop `incoming_client_tx`. but `ping_client_tx` was still alive in the ping task, so `client_rx` stayed open and `client_loop` kept running. the client kept receiving data. this is correct for grpc bidirectional streaming, half-closing one direction should not tear down the other. after the patch, `incoming_client_tx` is the only sender. when the incoming task breaks on `Ok(None)`, all senders are gone, `client_rx.recv()` returns `None`, and `client_loop` treats that as a disconnect. the entire connection is torn down because the client closed its send half. this breaks any client that sends its initial filter and then drops the send side of the stream, which is a common pattern. most simple clients send one `SubscribeRequest` and just read. in grpc, a client half-close is not a disconnect, it just means the client is done sending. the server should keep streaming. the fix is in the `Ok(None)` handler of the incoming message task. instead of breaking immediately (which drops the sender and cascades into a full disconnect), the task now awaits `incoming_cancellation_token.cancelled()`. this keeps `incoming_client_tx` alive so `client_loop` continues running until an actual disconnect condition happens like server shutdown or stream error. * chore: update changelog * chore: add comments
WilfredAlmeida
pushed a commit
that referenced
this pull request
Feb 5, 2026
* fix(grpc): keep connection alive on client send half-close the ping patch (`91709fd`, `str-300: do no skip ping logic even if the channel is full`) removed `ping_client_tx`, which was a clone of `client_tx` that lived inside the ping task. it was removed because pings no longer needed to signal disconnect explicitly, the new `.send().await` approach just breaks on error. removing that clone had a side effect. before the patch there were two senders for the `client_rx` channel: `ping_client_tx` in the ping task, and `incoming_client_tx` in the incoming message task. an `mpsc` channel only closes when all senders are dropped. when a client half-closed its send stream (normal in grpc, it just means "i have nothing more to send"), the incoming message task would receive `Ok(None)`, break, and drop `incoming_client_tx`. but `ping_client_tx` was still alive in the ping task, so `client_rx` stayed open and `client_loop` kept running. the client kept receiving data. this is correct for grpc bidirectional streaming, half-closing one direction should not tear down the other. after the patch, `incoming_client_tx` is the only sender. when the incoming task breaks on `Ok(None)`, all senders are gone, `client_rx.recv()` returns `None`, and `client_loop` treats that as a disconnect. the entire connection is torn down because the client closed its send half. this breaks any client that sends its initial filter and then drops the send side of the stream, which is a common pattern. most simple clients send one `SubscribeRequest` and just read. in grpc, a client half-close is not a disconnect, it just means the client is done sending. the server should keep streaming. the fix is in the `Ok(None)` handler of the incoming message task. instead of breaking immediately (which drops the sender and cascades into a full disconnect), the task now awaits `incoming_cancellation_token.cancelled()`. this keeps `incoming_client_tx` alive so `client_loop` continues running until an actual disconnect condition happens like server shutdown or stream error. * chore: update changelog * chore: add comments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
the ping patch #662 (
91709fd,str-300: do no skip ping logic even if the channel is full) removedping_client_tx, which was a clone ofclient_txthat lived inside the ping task. it was removed because pings no longer needed to signal disconnect explicitly, the new.send().awaitapproach just breaks on error.removing that clone had a side effect. before the patch there were two senders for the
client_rxchannel:ping_client_txin the ping task, andincoming_client_txin the incoming message task. anmpscchannel only closes when all senders are dropped. when a client half-closed its send stream (normal in grpc, it just means "i have nothing more to send"), the incoming message task would receiveOk(None), break, and dropincoming_client_tx. butping_client_txwas still alive in the ping task, soclient_rxstayed open andclient_loopkept running. the client kept receiving data. this is correct for grpc bidirectional streaming, half-closing one direction should not tear down the other.after the patch,
incoming_client_txis the only sender. when the incoming task breaks onOk(None), all senders are gone,client_rx.recv()returnsNone, andclient_looptreats that as a disconnect. the entire connection is torn down because the client closed its send half.this breaks any client that sends its initial filter and then drops the send side of the stream, which is a common pattern. most simple clients send one
SubscribeRequestand just read. in grpc, a client half-close is not a disconnect, it just means the client is done sending. the server should keep streaming.the fix is in the
Ok(None)handler of the incoming message task. instead of breaking immediately (which drops the sender and cascades into a full disconnect), the task now awaitsincoming_cancellation_token.cancelled(). this keepsincoming_client_txalive soclient_loopcontinues running until an actual disconnect condition happens like server shutdown or stream error.