bzs1118 commented on code in PR #3480:
URL: https://github.com/apache/brpc/pull/3480#discussion_r3843832333


##########
src/brpc/rdma/rdma_endpoint.cpp:
##########
@@ -916,16 +924,30 @@ ssize_t RdmaEndpoint::HandleCompletion(ibv_wc& wc) {
     }
     case IBV_WC_RECV: {  // recv completion
         // Please note that only the first wc.byte_len bytes is valid
+        ssize_t bytes_written = 0;
         if (wc.byte_len > 0) {
             if (wc.byte_len < (uint32_t)FLAGS_rdma_zerocopy_min_size) {
                 zerocopy = false;
             }
-            CHECK_NE(_state.load(butil::memory_order_relaxed), FALLBACK_TCP);
-            if (zerocopy) {
-                _rbuf[_rq_received].cutn(&_socket->_read_buf, wc.byte_len);
+            // Don't write to _read_buf until the handshake is fully done
+            // (ESTABLISHED). During the handshake (S_ACK_WAIT etc.), the
+            // main socket's OnNewMessages is driving the handshake via
+            // _read_buf; PollCq writing to _read_buf concurrently corrupts
+            // the IOBuf (non-thread-safe). Fall through to handle imm
+            // data, re-post recv WR, and send ack normally.
+            if (_state.load(butil::memory_order_acquire) != ESTABLISHED) {
+                LOG(WARNING) << "RDMA recv completion in non-ESTABLISHED state 
"
+                             << GetStateStr() << ", drop "
+                             << wc.byte_len << " bytes from "
+                             << _socket->description();
             } else {
-                // Copy data when the receive data is really small
-                _socket->_read_buf.append(_rbuf_data[_rq_received], 
wc.byte_len);
+                if (zerocopy) {
+                    _rbuf[_rq_received].cutn(&_socket->_read_buf, wc.byte_len);
+                } else {
+                    // Copy data when the receive data is really small
+                    _socket->_read_buf.append(_rbuf_data[_rq_received], 
wc.byte_len);
+                }
+                bytes_written = wc.byte_len;
             }

Review Comment:
   The race window is between BringUpQp (QP→RTS) and state.store(ESTABLISHED) — 
the server only needs to process the 4-byte ACK (one cutn call),  So at most 
0–1 RDMA messages arrive(or lost). Buffer the data outside _read_buf could add 
significant complexity.
   Plus, baidu_std is request-response: the client detects the missing response 
via timeout and retries. The retry succeeds because the server is now 
ESTABLISHED.



-- 
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]

Reply via email to