legionxiong opened a new issue, #3405:
URL: https://github.com/apache/brpc/issues/3405
## Description
There is a race when an RDMA connection falls back to TCP. The fallback is
represented by two separate fields:
- `RdmaEndpoint::_state = FALLBACK_TCP`
- `Socket::_rdma_state = RDMA_OFF`
Some fallback paths publish these fields in the following order:
```cpp
ep->_state = FALLBACK_TCP;
s->_rdma_state = Socket::RDMA_OFF;
```
Meanwhile, the TCP event path uses `RdmaEndpoint::_state` to decide whether
it
can dispatch normal TCP reads:
```cpp
void RdmaEndpoint::OnNewDataFromTcp(Socket* socket) {
// ...
if (ep->_state == FALLBACK_TCP) {
InputMessenger::OnNewMessages(socket);
return;
}
}
```
`InputMessenger::OnNewMessages()` can eventually call `Socket::DoRead()`,
which requires the socket-level RDMA state to have already transitioned to
`RDMA_OFF`:
```cpp
CHECK(_rdma_state == RDMA_OFF);
```
A concurrent TCP event can therefore observe `FALLBACK_TCP` before
`_rdma_state` has been updated and trigger the CHECK.
In addition, `RdmaEndpoint::_state` is currently a non-atomic field despite
being accessed by the RDMA handshake bthread, TCP event handling, and RDMA
completion handling.
## Observed failure
The process aborts at:
```cpp
ssize_t Socket::DoRead(size_t size_hint) {
// ...
CHECK(_rdma_state == RDMA_OFF);
return _read_buf.append_from_file_descriptor(fd(), size_hint);
}
```
The affected downstream binary was built from a branch containing commit
`53359cb6`.
## Race sequence
```mermaid
sequenceDiagram
participant H as RDMA handshake bthread
participant E as TCP event bthread
participant M as InputMessenger
participant S as Socket::DoRead
Note over H: RDMA negotiation fails<br/>and starts TCP fallback
H->>H: ep->_state = FALLBACK_TCP
Note over H,E: H may be descheduled here
E->>E: OnNewDataFromTcp(socket)
E->>E: observe ep->_state == FALLBACK_TCP
E->>M: InputMessenger::OnNewMessages(socket)
M->>S: Socket::DoRead(size_hint)
S->>S: CHECK(_rdma_state == RDMA_OFF)
Note over S: _rdma_state may still be RDMA_UNKNOWN/RDMA_ON<br/>CHECK
fails and the process aborts
H->>H: s->_rdma_state = RDMA_OFF
Note over H: This update happens too late
```
Equivalent thread interleaving:
```text
RDMA handshake bthread TCP event bthread
---------------------- -----------------
ep->_state = FALLBACK_TCP;
< context switch >
OnNewDataFromTcp(socket);
if (ep->_state == FALLBACK_TCP) {
InputMessenger::OnNewMessages(socket);
-> Socket::DoRead();
-> CHECK(_rdma_state ==
RDMA_OFF);
// failure
}
s->_rdma_state = RDMA_OFF;
```
`TryReadOnTcp()` itself is not the problematic ordering path. When it is
called by the handshake bthread after both assignments, program order is
preserved. The race is caused by an independently scheduled
`OnNewDataFromTcp()` invocation observing the partially published fallback
state.
## Expected behavior
Once a TCP event observes `RdmaEndpoint::_state == FALLBACK_TCP`, the
corresponding `Socket::_rdma_state` must already be visible as `RDMA_OFF`.
TCP fallback must not expose an intermediate state in which:
```text
RdmaEndpoint::_state == FALLBACK_TCP
Socket::_rdma_state != RDMA_OFF
```
--
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]
