chenBright opened a new pull request, #3691:
URL: https://github.com/apache/brpc/pull/3691
### What problem does this PR solve?
Issue Number: resolve
Problem Summary:
- Pooled and short sockets omit per-socket connection options, causing
connections to use
global defaults.
- Removing a failed socket can race with health-check revival and leak an
additional reference.
```
The leak occurs because removal assumes the additional reference is already
gone, but health
checking restores it afterward, leaving nobody responsible for releasing it.
Let:
- A = additional reference.
- H = the owner’s health-check reference.
- T = the health-check task’s reference.
Initially, the socket is already failed. SetFailed() has released A, leaving
H + T = 2.
Removal thread Health-check thread
References
| |
| ReleaseAdditionalReference() |
| A was already released; return | 2 = H +
T
| |
| H has not been released yet | Revive(2)
| | Reference count is 2
| | Revive and restore A 3 = H +
T + A
| |
| ReleaseHCRelatedReference() |
| Release H | 2 = T +
A
| |
| | Health-check task ends
| | Release T 1 = A
v v
The owner has removed the socket, and the health-check task has exited.
Nobody releases A, so the reference count never reaches zero.
The original state only recorded that A had been released. It did not
distinguish temporary
failure from permanent removal. A reference count of 2 does not establish
that the owner
still wants the socket
```
### What is changed and the side effects?
Changed:
- Centralize sub-socket option construction and propagate keepalive
settings, TCP user timeout,
and bthread tag.
- Distinguish references released by `SetFailed()` from permanent owner
releases. Coordinate
removal with revival and update SelectiveChannel cleanup accordingly.
```
The fix introduces separate states: ADDITIONAL_REF_RELEASED_ON_FAILURE
allows revival,
while ADDITIONAL_REF_RELEASED permanently prevents it.
Removal thread Health-check thread
References
| |
| ReleaseAdditionalReference() |
| RELEASED_ON_FAILURE -> RELEASED |
| Mark permanent removal |
| No second reference decrement | 2 = H +
T
| |
| | Revive(2)
| | CAS to REVIVING fails
| | Observe RELEASED
| | Return without adding A
| | 2 = H +
T
| Release H | 1 = T
| | Release T 0
v v
The reference count reaches zero, and the socket is recycled.
If revival acquires REVIVING first, removal waits for it to finish, then
marks permanent removal
and releases the restored A. Both orderings avoid the leak.
```
Side effects:
- Performance effects:
- Breaking backward compatibility:
---
### Check List:
- Please make sure your changes are compilable.
- When providing us with a new feature, it is best to add related tests.
- Please follow [Contributor Covenant Code of
Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).
--
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]
