Copilot commented on code in PR #3507:
URL: https://github.com/apache/brpc/pull/3507#discussion_r3889570777
##########
src/brpc/ubshm/ub_endpoint.cpp:
##########
@@ -580,15 +672,17 @@ void* UBShmEndpoint::ProcessHandshakeAtServer(void* arg) {
uint32_t* tmp = (uint32_t*)data;
uint32_t flags = butil::NetToHost32(*tmp);
if (flags & ACK_MSG_UB_OK) {
- if (ub_transport->_ub_state == UBShmTransport::UB_OFF) {
- LOG(WARNING) << "Fail to parse Hello Message length from client:"
+ if (ub_transport->_ub_state == UBShmTransport::UB_OFF ||
+ selected_format == UBR_DATA_FORMAT_NONE) {
+ LOG(WARNING) << "Invalid successful ACK from client:"
<< s->description();
s->SetFailed(EPROTO, "Fail to complete ub handshake from %s: %s",
s->description().c_str(), berror(EPROTO));
Review Comment:
When the server ends up falling back to TCP (ACK=0), any UB resources
allocated earlier in this handshake (shared memory mappings, poller socket,
timers) are kept until the socket is destroyed. For long-lived TCP connections,
this can accumulate significant unused shared-memory resources. Consider
releasing UB resources when entering `FALLBACK_TCP`.
##########
src/brpc/ubshm/ub_endpoint.cpp:
##########
@@ -442,6 +498,7 @@ void* UBShmEndpoint::ProcessHandshakeAtClient(void* arg) {
}
if (ub_transport->_ub_state == UBShmTransport::UB_ON) {
+ ep->_negotiated_data_format = selected_format;
ep->_state = ESTABLISHED;
ep->_ub_ring->UbrUnlinkLocalShm();
LOG_IF(INFO, FLAGS_ub_trace_verbose)
Review Comment:
If the client falls back to TCP (e.g., negotiating with a V2 peer or format
mismatch), `_ub_ring` and the allocated shared memory from
`AllocateClientResources()` remain alive for the lifetime of the TCP
connection. This can waste large shared-memory segments per connection even
though UBRing is disabled. Consider explicitly releasing UB resources before
transitioning to `FALLBACK_TCP`.
--
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]
