Skip to content

Commit 01357bf

Browse files
committed
async/client: don't spawn per incoming frame
ClientReader::handle_msg spawns a new task per frame. For a server-streaming RPC, the final DATA frame and the subsequent FLAG_REMOTE_CLOSED frame then race: if the close-frame task grabs the req_map lock first, it removes the stream from the map, and the preceding data-frame task finds nothing and silently drops the payload. The stream consumer sees Ok(None) (EOF) without ever observing the payload the server sent. The connection read loop already awaits handle_msg per frame, so processing inline preserves per-stream wire order. It also gives the per-stream mpsc natural back-pressure in place of the unbounded per-frame spawning. handle_err gets the same treatment for consistency. Also update example/async-stream-client.rs so the existing echo_default_value example exercises the race: wrap it in a 1000-iteration loop and flip the client runtime to multi_thread. On a single-threaded runtime the tokio scheduler masks the race because spawned tasks run in submission order. Without the fix, the modified example fails on any multi-core runner; with the fix, it passes. Signed-off-by: Shiv Bhosale <shvbsle@amazon.com>
1 parent f31f592 commit 01357bf

1 file changed

Lines changed: 17 additions & 16 deletions

File tree

src/asynchronous/client.rs

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -313,26 +313,27 @@ impl ReaderDelegate for ClientReader {
313313
async fn exit(&self) {}
314314

315315
async fn handle_err(&self, header: MessageHeader, e: Error) {
316+
// Process inline to preserve per-stream frame ordering. See `handle_msg`.
316317
let req_map = self.streams.clone();
317-
tokio::spawn(async move {
318-
if let Some(resp_tx) = get_resp_tx(req_map, &header).await {
319-
resp_tx
320-
.send(Err(e))
321-
.await
322-
.unwrap_or_else(|_e| error!("The request has returned"));
323-
}
324-
});
318+
if let Some(resp_tx) = get_resp_tx(req_map, &header).await {
319+
resp_tx
320+
.send(Err(e))
321+
.await
322+
.unwrap_or_else(|_e| error!("The request has returned"));
323+
}
325324
}
326325

327326
async fn handle_msg(&self, msg: GenMessage) {
327+
// Do not `tokio::spawn` per frame: a `FLAG_REMOTE_CLOSED` frame could
328+
// then `remove` a stream from `req_map` before the preceding DATA
329+
// frame's task looked it up, silently dropping the final payload.
330+
// The read loop already awaits this per frame, so inline is correct.
328331
let req_map = self.streams.clone();
329-
tokio::spawn(async move {
330-
if let Some(resp_tx) = get_resp_tx(req_map, &msg.header).await {
331-
resp_tx
332-
.send(Ok(msg))
333-
.await
334-
.unwrap_or_else(|_e| error!("The request has returned"));
335-
}
336-
});
332+
if let Some(resp_tx) = get_resp_tx(req_map, &msg.header).await {
333+
resp_tx
334+
.send(Ok(msg))
335+
.await
336+
.unwrap_or_else(|_e| error!("The request has returned"));
337+
}
337338
}
338339
}

0 commit comments

Comments
 (0)