Copilot commented on code in PR #3531:
URL: https://github.com/apache/brpc/pull/3531#discussion_r3953848944
##########
test/brpc_streaming_rpc_unittest.cpp:
##########
@@ -1446,8 +1447,11 @@ TEST_F(StreamingRpcTest,
limit_streams_accepted_per_request) {
std::memory_order_acquire));
}
+ for (brpc::StreamId stream_id : service.response_streams) {
+ ASSERT_EQ(0, brpc::StreamClose(stream_id));
+ }
for (brpc::StreamId stream_id : request_streams) {
- brpc::StreamClose(stream_id);
+ ASSERT_EQ(0, brpc::StreamClose(stream_id));
}
Review Comment:
Stream cleanup is performed after multiple ASSERT_* checks. If any assertion
fails earlier in the loop iteration, the test returns immediately and the
streams stay open, which can reintroduce hangs/leaks during failure
diagnostics. Consider adding RAII guards (brpc::ScopedStream) for both request
and response stream IDs so they get closed even on early-return, while still
asserting StreamClose succeeds by releasing the guards after explicit closes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
